|
(#398)
* Restricted HTTP header field lookup to the header section
_nx_web_http_server_field_value_get() searched the whole request packet for
header fields. Per RFC 7230 header parsing must stop at the first CRLFCRLF: the
message body is opaque and must never be interpreted as header fields. Because
the scan ran past that terminator, a field name appearing in the body was
reported as a header field, so a request declaring Content-Length whose body
contained "Transfer-Encoding: chunked" made
_nx_web_http_server_chunked_check() see chunked encoding.
Bound the scan to the header section and track whether the field was found in an
explicit flag. The flag matters: the absent-field outcome used to be inferred
from the scan pointer having run past the end of the packet, so bounding the scan
without it left that outcome undetectable and the value was copied from a fixed
offset inside the body instead.
The whole field name must lie inside the header, so a name is not matched across
the boundary. A packet with no header terminator carries no body, so all of it is
scanned.
Reported and first patched by Jannes Wegner in issue #397; the implementation was
reworked during review, and a regression test was added covering the reported
request, a body beginning with the field name, an absent field whose body holds a
CRLF, a packet with no terminator, and a header field competing with a body
decoy.
* Fixed an out-of-bounds read in the header value whitespace skip
The whitespace skip in _nx_web_http_server_field_value_get() read the character
before testing the bound, so a header value made up entirely of spaces running to
the last byte of the packet data caused one read at nx_packet_append_ptr. The
check inside the loop body then returned NX_WEB_HTTP_NOT_FOUND, so the outcome
was correct but the read had already happened.
Test the bound in the loop condition instead. Running off the end is still caught
immediately afterwards by the CRLF bound check, so the returned status is
unchanged; only the read is removed.
Confirmed with AddressSanitizer against an exact-size buffer: heap-buffer-overflow
READ of size 1 before, clean after, NX_WEB_HTTP_NOT_FOUND in both cases.
This defect predates the header boundary fix in the preceding commit and is
independent of it, so it is kept separate to remain cherry-pickable on its own.
---------
Co-authored-by: Frédéric Desbiens <[email protected]>
Assisted-by: Claude Opus 5 (1M context) <[email protected]>
|
|
* Fix: in the Web add-on, packets may be incorrectly released in case of errors
* Added chunked packet release to the PUT underflow error paths
The two Content-Length underflow branches in _nx_web_http_server_put_process
were the only error exits in that function that released nothing at all. Every
neighbouring exit releases the request packet when the request is chunked.
A chunked request cannot currently reach either branch, because length is
initialised from nx_packet_length so the subtraction cannot wrap. Relying on
that invariant for correctness is fragile, so release the packet the same way
the surrounding paths do.
Assisted-by: Claude Opus 5 (1M context) <[email protected]>
* Added a regression test for the PUT Content-Length underflow path
The underflow detection in _nx_web_http_server_put_process had no coverage. A
raw TCP client now sends a PUT whose Content-Length is smaller than the body it
carries, and the test checks that the server answers 400 and that its packet
pool is fully restored afterwards.
Registered in the web regression CMake list and in netx_https_testcontrol.
Verified against default_build_coverage, no_tls_build_coverage and
digest_authenticate_build: 54/54 pass in each, no new warnings under -Werror.
---------
Co-authored-by: Frédéric Desbiens <[email protected]>
Assisted-by: Claude Code (Opus 5) <[email protected]>
|
