actix-web icon indicating copy to clipboard operation
actix-web copied to clipboard

lack validation of whether request payload match content-length

Open hxzhao527 opened this issue 2 years ago • 1 comments

Steps to Reproduce (for bugs)

code

use actix_web::{post, web, App, HttpServer};
use serde::Deserialize;

use log::warn;

#[derive(Deserialize)]
struct Info {
    name: String,
}

#[post("/")]
async fn index(web::Form(form): web::Form<Info>) -> String {
    warn!("name length: {}", form.name.len());
    format!("Welcome {}!", form.name)
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    std::env::set_var("RUST_LOG", "debug");
    env_logger::init();

    HttpServer::new(|| {
        let form_config = web::FormConfig::default()
            .limit(4096)
            .error_handler(|err, _| {
                warn!("Form error: {}", err);
                actix_web::error::InternalError::from_response(
                    err,
                    actix_web::HttpResponse::Conflict().finish(),
                )
                .into()
            });
        App::new()
            .app_data(form_config)
            .service(index)
            .wrap(actix_web::middleware::Logger::default())
    })
    .bind(("127.0.0.1", 1234))?
    .run()
    .await
}
[dependencies]
actix-web = "4.3.1"
env_logger = "0.10.0"
log = "0.4.19"
serde = { version = "1.0", features = ["derive"] }

steps

  1. cargo run
  2. send a request with content-length larger thn body
curl -v --location 'http://127.0.0.1:1234' --header 'Content-Length: 1000' --header 'Content-Type: application/x-www-form-urlencoded' --data-urlencode 'name=haha'
  1. ctrl-c stop the curl

Current Behavior

❯ cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/rust_study`
[2023-07-08T01:59:18Z INFO  actix_server::builder] starting 8 workers
[2023-07-08T01:59:18Z INFO  actix_server::server] Actix runtime found; starting in Actix runtime
[2023-07-08T01:59:26Z WARN  rust_study] name length: 4
[2023-07-08T01:59:26Z INFO  actix_web::middleware::logger] 127.0.0.1 "POST / HTTP/1.1" 200 13 "-" "curl/8.1.2" 3.468221

Expected Behavior

  1. log something like Form error: incomplete request
  2. http-status code 400 not 200

Context

rfc-2616 When a Content-Length is given in a message where a message-body is allowed, its field value MUST exactly match the number of OCTETs in the message-body. HTTP/1.1 user agents MUST notify the user when an invalid length is received and detected.

the incomplete request should not be handled as normal. because:

  1. not match http sepc
  2. the handler does not have the Content-Length, it can not valid this by itself.

Possible Solution

take form as an example, check payload length after got https://github.com/actix/actix-web/blob/ce3af777a05e6be2673290417b25f714e6e25830/actix-web/src/types/form.rs#L383-L396

hxzhao527 avatar Jul 08 '23 02:07 hxzhao527

I encountered this issue last week. I'm just about to open an issue, and I just write here.

After some investigating I think the core problem is at Payload status passing. I submitted an PR #3068 to fixed. With your demo code, the output becomes:

[2023-07-10T07:35:53Z INFO  actix_server::builder] starting 32 workers
[2023-07-10T07:35:53Z INFO  actix_server::server] Actix runtime found; starting in Actix runtime
[2023-07-10T07:35:58Z WARN  demo] Form error: Error that occur during reading payload: payload reached EOF before completing: Some(Kind(UnexpectedEof)).
[2023-07-10T07:35:58Z DEBUG actix_web::middleware::logger] Error in response: Payload(Incomplete(Some(Kind(UnexpectedEof))))
[2023-07-10T07:35:58Z INFO  actix_web::middleware::logger] 127.0.0.1 "POST / HTTP/1.1" 409 0 "-" "curl/8.1.2" 3.143109

ihciah avatar Jul 10 '23 07:07 ihciah