Skip to content
This repository has been archived by the owner on Nov 6, 2022. It is now read-only.

Content-Length with obsolete line folding is accepted with invalid input #456

Closed
obatysh opened this issue Dec 10, 2018 · 6 comments
Closed

Comments

@obatysh
Copy link

obatysh commented Dec 10, 2018

According to https://tools.ietf.org/html/rfc7230#section-3.2.4
requests/responses with obsolete line folding can be rejected or obs-fold can be treated as one or more spaces.
obs-fold = CRLF 1*( SP / HTAB )

Currently any input after an obsolete line folding in Content-Length field would be accepted.
Example request:
"POST / HTTP/1.1\r\n"
"Content-Length: 42\r\n"
" ANY TEXT\r\n" // would be accepted and reported as part of Content-Length header field value
"\r\n"
...

@indutny
Copy link
Member

indutny commented Dec 12, 2018

I think this should yield HPE_UNEXPECTED_CONTENT_LENGTH error. Is there a test case that would demonstrate that it behaves differently?

Thanks for reporting this!

@obatysh
Copy link
Author

obatysh commented Dec 12, 2018

There is no test yet, and it was reporting error before the commit 01da95f was introduced.

The issue happens because after obsolete line folding is discovered the parser goes back to s_header_value_start. But as new content length states h_content_length_num, h_content_length_ws are not handled there it goes to default case and h_general state.

@indutny
Copy link
Member

indutny commented Dec 12, 2018

Thank you for checking it. Your analysis of the code path is absolutely correct! Did I understand it right, that after 01da95f the bug is no longer reproducible?

@obatysh
Copy link
Author

obatysh commented Dec 12, 2018

No, vice versa, the bug appears after
01da95f

@indutny
Copy link
Member

indutny commented Dec 12, 2018

@obatysh would you care to clarify if this error is actually reproducible and you experienced it?

@indutny
Copy link
Member

indutny commented Dec 12, 2018

Nevermind, I can reproduce it now.

ploxiln pushed a commit to ploxiln/http-parser that referenced this issue Jan 4, 2019
Content-Length with line folding was accepted with invalid input. Treat
obsolete line folding as space and continue parsing

Fixes: nodejs#456
PR-URL: nodejs#458
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants