Malformed HTTP/2 may cause 'assertion failed' #1994

heikki-heikkila · 2021-11-26T12:43:55Z

Description

With a certain malformed HTTP request, a HTTP server that runs Rocket will panic with 'assertion failed' (details below). The whole HTTP server does not crash, however; the server SW catches the panic and keeps running.

This should be seen as a bug, because 'assertion failed' should never happen in production software.

To Reproduce

The issue can be reproduced in a crate with three files, src/server.rs, src/client.rs and Cargo.toml, as follows. See instructions after the file listings.

// server.rs
// A very simple textbook server

// Instructions for use:
// Run "cargo run --bin http_server" in one command window. The HTTP
// server starts and waits for requests.
// In another command window, run "cargo run --bin http_client". The client
// should send one query to the server and print the outcome.

// The server runs in debug mode and prints useful diagnostics to the
// terminal screen.

#[macro_use] extern crate rocket;

#[get("/world")]
fn world() -> &'static str {
"Hello, world!"
}

#[launch]
fn rocket() -> _ {
rocket::build().mount("/hello", routes![world])
}
// end of file server.rs

// client.rs
// A very simple HTTP client that produces an invalid HTTP request.
// In "path and query", where slash '/' is supposed to be a separator,
// we do erroneous percent encoding "/" -> "%2F", just to see what the
// HTTP server does.

// Instructions for use:
// Run "cargo run --bin http_server" in one command window. The HTTP
// server starts and waits for requests.
// In another command window, run "cargo run --bin http_client". The client
// should send one query to the server and print the outcome.

#[tokio::main]

async fn main() {

// create the http client system
let client : hyper::Client<hyper::client::connect::HttpConnector> =
                hyper::Client::builder()
                  .http2_only(true)
                  .build_http();

let uri = if let Ok(x)=
                        http::uri::Builder::new()
                            .scheme("http")
                            .authority("127.0.0.1:8000")
                             // the correct string would be
                             // "/hello/world":
                            .path_and_query("%2Fhello%2Fworld")
                            .build()
          { x } else { return; };

let req : http::request::Request<hyper::body::Body> =
          if let Ok(x) =
                        hyper::Request::builder()
                            .method(http::method::Method::GET)
                            .uri(uri)
                            .body(hyper::body::Body::from(""))
          { x } else { return; };

match client.clone().request(req).await {
    Ok(x) => {
        println!("Success {:?}", x);
    }
    Err(e) => {
        println!("Failure {:?}", e);
    }
}

}
// end of file client.rs

Cargo.toml

[package]
name = "http_panic_test"
version = "0.1.0"
edition = "2021"
authors = ["H. Heikkilä"]
description = "Test panic condition in Rocket HTTP server"

See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
http = "0.2.5"
mac_address = "1.1.2"
rocket = "0.5.0-rc.1"
futures = "0.3"
hyper = { version = "0.14.13", features = [
"http2", "server", "client", "tcp" ] }
tokio = { version = "1.12.0", features = [
"sync", "rt", "net", "time" ] }

[[bin]]
name = "http_server"
path = "src/server.rs"

[[bin]]
name = "http_client"
path = "src/client.rs"

end of Cargo.toml

Instructions for reproducing the issue:

Set up a cargo crate with files ./src/server.rs, ./src/client.rs and ./Cargo.toml
Build SW as usual: cargo build
Open one command window and do "cargo run --bin http_server". This starts an HTTP server, which starts waiting for requests. (The server reserves socket address 128.0.0.1:8000.)
Open another command window and do "cargo run --bin http_client". This runs an HTTP client, which sends one request towards the HTTP server.
In the HTTP server window, a message like the following is seen:

thread 'rocket-worker-thread' panicked at 'assertion failed: Origin::parse(uri.as_str()).is_ok()', . . . cache/registry/src/github.com-1ecc6299db9ec823/rocket-0.5.0-rc.1/src/request/request.rs:846:9
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace

Expected Behavior

I expect that HTTP servers are robust, so that 'assertion failed' never happens, even if the HTTP request message is malformed or perhaps even malicious. (Admittedly, the HTTP server catches the assertion gracefully.)

Environment:

OS Distribution and Kernel: Linux ubuntu-14-04
Rocket Version: 0.5.0-rc.1

Additional Context

I discovered this behaviour by chance. I was doing percent encoding to HTTP request in client SW -- which is required in some cases -- but erroneously applied percent encoding to the slash separators.

The text was updated successfully, but these errors were encountered:

IniterWorker · 2021-12-19T02:11:34Z

It seems related to this. I am wondering what is the correct HTTP error status for this purpose.

// In debug, make sure we agree with Hyper. Otherwise, cross our fingers
// and trust that it only gives us valid URIs like it's supposed to.
// TODO: Keep around not just the path/query, but the rest, if there?
let uri = hyper.uri.path_and_query().ok_or(Error::InvalidUri(&hyper.uri))?;
debug_assert!(Origin::parse(uri.as_str()).is_ok());
let uri = Origin::new(uri.path(), uri.query().map(Cow::Borrowed));

Should we report it to the Hyper issue tracker and maybe return an enum Error<'r> on the Rocket side to remove the assert?

IniterWorker · 2021-12-19T02:14:53Z

Related to #1831

kolbma · 2022-01-16T15:32:51Z

What is here the difference between HTTP/1.1 and 2 in hyper?
The debug_assert fails only with HTTP/2.

https://github.com/SergioBenitez/Rocket/blob/8cae077ba1d54b92cdef3e171a730b819d5eeb8e/core/lib/src/request/request.rs#L974-L979

I'm asking because I am not sure at the moment what would be the correct handling...
Is it a BadRequest (like it is currently for HTTP/1.1) or a NotFound?

[src/bin/client.rs:33] &req = Request {
    method: GET,
    uri: http://127.0.0.1:8000/hello/world,
    version: HTTP/1.1,
    headers: {},
    body: Body(
        Empty,
    ),
}
[src/bin/client.rs:35] &res = Response {
    status: 200,
    version: HTTP/2.0,
    headers: {
        "content-type": "text/plain; charset=utf-8",
        "server": "Rocket",
        "x-content-type-options": "nosniff",
        "x-frame-options": "SAMEORIGIN",
        "permissions-policy": "interest-cohort=()",
        "content-length": "13",
        "date": "Sun, 16 Jan 2022 15:36:27 GMT",
    },
    body: Body(
        Streaming,
    ),
}
[src/bin/client.rs:40] &req = Request {
    method: GET,
    uri: http://127.0.0.1:8000/%2Fhello%2Fworld,
    version: HTTP/1.1,
    headers: {},
    body: Body(
        Empty,
    ),
}
[src/bin/client.rs:42] &res = Response {
    status: 404,
    version: HTTP/2.0,
    headers: {
        "content-type": "text/html; charset=utf-8",
        "server": "Rocket",
        "x-content-type-options": "nosniff",
        "x-frame-options": "SAMEORIGIN",
        "permissions-policy": "interest-cohort=()",
        "content-length": "383",
        "date": "Sun, 16 Jan 2022 15:36:27 GMT",
    },
    body: Body(
        Streaming,
    ),
}
[src/bin/client.rs:58] &req = Request {
    method: GET,
    uri: http://127.0.0.1:8000/%2Fhello%2Fworld,
    version: HTTP/1.1,
    headers: {},
    body: Body(
        Empty,
    ),
}
[src/bin/client.rs:61] &res = Response {
    status: 404,
    version: HTTP/2.0,
    headers: {
        "content-type": "text/html; charset=utf-8",
        "server": "Rocket",
        "x-content-type-options": "nosniff",
        "x-frame-options": "SAMEORIGIN",
        "permissions-policy": "interest-cohort=()",
        "content-length": "383",
        "date": "Sun, 16 Jan 2022 15:36:27 GMT",
    },
    body: Body(
        Streaming,
    ),
}
[src/bin/client.rs:80] &req = Request {
    method: GET,
    uri: http://127.0.0.1:8000%2Fhello%2Fworld,
    version: HTTP/1.1,
    headers: {},
    body: Body(
        Empty,
    ),
}
Error: hyper::Error(Http2, Error { kind: Reset(StreamId(7), INTERNAL_ERROR, Remote) })

With HTTP/1.1...

[src/bin/client.rs:34] &req = Request {
    method: GET,
    uri: http://127.0.0.1:8000/hello/world,
    version: HTTP/1.1,
    headers: {},
    body: Body(
        Empty,
    ),
}
[src/bin/client.rs:36] &res = Response {
    status: 200,
    version: HTTP/1.1,
    headers: {
        "content-type": "text/plain; charset=utf-8",
        "server": "Rocket",
        "x-content-type-options": "nosniff",
        "x-frame-options": "SAMEORIGIN",
        "permissions-policy": "interest-cohort=()",
        "content-length": "13",
        "date": "Sun, 16 Jan 2022 15:41:22 GMT",
    },
    body: Body(
        Streaming,
    ),
}
[src/bin/client.rs:41] &req = Request {
    method: GET,
    uri: http://127.0.0.1:8000/%2Fhello%2Fworld,
    version: HTTP/1.1,
    headers: {},
    body: Body(
        Empty,
    ),
}
[src/bin/client.rs:43] &res = Response {
    status: 404,
    version: HTTP/1.1,
    headers: {
        "content-type": "text/html; charset=utf-8",
        "server": "Rocket",
        "x-content-type-options": "nosniff",
        "x-frame-options": "SAMEORIGIN",
        "permissions-policy": "interest-cohort=()",
        "content-length": "383",
        "date": "Sun, 16 Jan 2022 15:41:22 GMT",
    },
    body: Body(
        Streaming,
    ),
}
[src/bin/client.rs:59] &req = Request {
    method: GET,
    uri: http://127.0.0.1:8000/%2Fhello%2Fworld,
    version: HTTP/1.1,
    headers: {},
    body: Body(
        Empty,
    ),
}
[src/bin/client.rs:62] &res = Response {
    status: 404,
    version: HTTP/1.1,
    headers: {
        "content-type": "text/html; charset=utf-8",
        "server": "Rocket",
        "x-content-type-options": "nosniff",
        "x-frame-options": "SAMEORIGIN",
        "permissions-policy": "interest-cohort=()",
        "content-length": "383",
        "date": "Sun, 16 Jan 2022 15:41:22 GMT",
    },
    body: Body(
        Streaming,
    ),
}
[src/bin/client.rs:81] &req = Request {
    method: GET,
    uri: http://127.0.0.1:8000%2Fhello%2Fworld,
    version: HTTP/1.1,
    headers: {},
    body: Body(
        Empty,
    ),
}
[src/bin/client.rs:84] &res = Response {
    status: 400,
    version: HTTP/1.1,
    headers: {
        "content-length": "0",
        "date": "Sun, 16 Jan 2022 15:41:22 GMT",
    },
    body: Body(
        Empty,
    ),
}

kolbma · 2022-01-16T17:59:21Z

"Funny" stuff...

hyperium/hyper#2736

kolbma · 2022-01-16T18:10:52Z

@heikki-heikkila Can you please edit the title and change the HTTP to HTTP/2!

Soham3-1415 · 2022-02-02T15:39:51Z

curl 'localhost:8000/a/\/b' fails at the same assertion with thread 'rocket-worker-thread' panicked at 'assertion failed: Origin::parse(uri.as_str()).is_ok()', .../.cargo/git/checkouts/rocket-8bf16d9ca7e90bdc/8cae077/core/lib/src/request/request.rs:978:9 using the following code:

// src/main.rs

#[macro_use]
extern crate rocket;

#[get("/")]
fn index() -> &'static str {
    "Hello, world!"
}

#[launch]
fn rocket() -> _ {
    rocket::build().mount("/", routes![index])
}

# Cargo.toml

[package]
name = "untitled"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
rocket = {git = "https://github.com/SergioBenitez/Rocket.git", rev = "8cae077ba1d54b92cdef3e171a730b819d5eeb8e"}

Command

curl -v 'localhost:8000/a/\/b'

Output

*   Trying 127.0.0.1:8000...
* Connected to localhost (127.0.0.1) port 8000 (#0)
> GET /a/\/b HTTP/1.1
> Host: localhost:8000
> User-Agent: curl/7.74.0
> Accept: */*
> 
* Empty reply from server
* Connection #0 to host localhost left intact
curl: (52) Empty reply from server

However, this is with HTTP/1.1 and not HTTP/2. I'm not sure if this is a related issue, the same issue, or an entirely different issue.

SergioBenitez · 2022-04-28T02:38:26Z

Closing in favor of #1831. See my comment in #1831 (comment) for an explanation of what's going on. In short: Hyper needs to be fixed.

heikki-heikkila added the triage A bug report being investigated label Nov 26, 2021

heikki-heikkila changed the title ~~Malformed HTTP may cause 'assertion failed'~~ Malformed HTTP/2 may cause 'assertion failed' Jan 17, 2022

kolbma mentioned this issue Jan 19, 2022

path_and_query allow path not starting with slash hyperium/http#507

Open

SergioBenitez added bug Deviation from the specification or expected behavior upstream An unresolvable issue: an upstream dependency bug and removed triage A bug report being investigated labels Apr 28, 2022

SergioBenitez closed this as completed Apr 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Malformed HTTP/2 may cause 'assertion failed' #1994

Malformed HTTP/2 may cause 'assertion failed' #1994

heikki-heikkila commented Nov 26, 2021

IniterWorker commented Dec 19, 2021

IniterWorker commented Dec 19, 2021

kolbma commented Jan 16, 2022 •

edited

Loading

kolbma commented Jan 16, 2022

kolbma commented Jan 16, 2022

Soham3-1415 commented Feb 2, 2022

SergioBenitez commented Apr 28, 2022

Malformed HTTP/2 may cause 'assertion failed' #1994

Malformed HTTP/2 may cause 'assertion failed' #1994

Comments

heikki-heikkila commented Nov 26, 2021

Cargo.toml

See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

end of Cargo.toml

IniterWorker commented Dec 19, 2021

IniterWorker commented Dec 19, 2021

kolbma commented Jan 16, 2022 • edited Loading

kolbma commented Jan 16, 2022

kolbma commented Jan 16, 2022

Soham3-1415 commented Feb 2, 2022

SergioBenitez commented Apr 28, 2022

kolbma commented Jan 16, 2022 •

edited

Loading