libhtp icon indicating copy to clipboard operation
libhtp copied to clipboard

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

Open socketpair opened this issue 9 years ago • 9 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'',
])

socketpair avatar Nov 28 '14 04:11 socketpair

Is this still an issue ?

catenacyber avatar Apr 25 '22 11:04 catenacyber

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.

socketpair avatar Apr 25 '22 11:04 socketpair

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

catenacyber avatar Apr 25 '22 11:04 catenacyber

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

socketpair avatar Apr 25 '22 11:04 socketpair

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 avatar Apr 25 '22 12:04 catenacyber

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

socketpair avatar Apr 25 '22 12:04 socketpair

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

catenacyber avatar Apr 25 '22 12:04 catenacyber

"Better" means much better performance.

socketpair avatar Apr 28 '22 12:04 socketpair

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

catenacyber avatar Apr 28 '22 12:04 catenacyber

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

catenacyber avatar Apr 07 '23 15:04 catenacyber

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

victorjulien avatar Apr 13 '23 08:04 victorjulien

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

As in https://github.com/OISF/libhtp/pull/388 ?

catenacyber avatar Apr 26 '23 15:04 catenacyber