OISF/libhtp

libhtp does not support "chunk-extension"s in chunked mode

socketpair opened this issue · 12 comments

See:
http://tools.ietf.org/html/rfc2616#section-3.6.1

i.e.: part of my code in python, where I construct special crafted query.

query = '\r\n'.join([
    b'POST /qwe.asd HTTP/1.1',
    b'Host: ' + TEST_HOST,
    b'Transfer-Encoding: chunked',
    b'Content-Encoding: gzip',
    b'Content-Type: Application/binary',
    b'',
    hex(len(chunk1))[2:].upper() + b'  ',
    chunk1,
    hex(len(chunk2))[2:].upper() + b' ',
    chunk2,
    b'0   ',  # + br'; qwe3=asd3; zxc3="rt\"y3"',   <--------  BUG HERE ---------
    b'trailehdr1: tralierdata1',
    b'trailehdr2: tralierdata2',
    b'',
    b'',
])

Is this still an issue ?

I don't know. I think yes. It's better to stop using libhtp. I would advice https://llhttp.org/ - is the fastest and fully RFC compliant HTTP FSM.

Thanks for the quick answer. Current Suricata project is to use rust for HTTP parsing...

Did anyone make a benchmark ? (Rust parser vs others) ?

Here is the ongoing work for the rust parser https://github.com/cccs-rtmorti/libhtp-rs
I do not think it is completed yet, so no benchmark yet, if that was your question

@catenacyber who should I mention in order him to look to the llhttp ? Possibly it's a better solution than rust ?

@victorjulien I guess...
What do you mean by "better" ?
For instance, I think we want something more relaxed RFC-compliant cf http-evader tests

"Better" means much better performance.

I bet llhttp is faster, but I do not think that is our first criteria, for instance security comes before I think.

htp_parse_chunked_length skips "junk" (that is how it considers extensions) after integer.
Tested on modifying

diff --git a/test/files/04-post-urlencoded-chunked.t b/test/files/04-post-urlencoded-chunked.t
index 1d72e71..29c265f 100644
--- a/test/files/04-post-urlencoded-chunked.t
+++ b/test/files/04-post-urlencoded-chunked.t
@@ -7,7 +7,7 @@ Cookie: 1
 
 b
 p=012345678
-1
+1 ; qwe3=asd3
 9
 0

should we just skip it? Or perhaps raise an event?

should we just skip it? Or perhaps raise an event?

As in #388 ?