Skip to content

tool_cb_hdr: add an additional parsing check#12320

Closed
jay wants to merge 1 commit into
curl:masterfrom
jay:hdrparse
Closed

tool_cb_hdr: add an additional parsing check#12320
jay wants to merge 1 commit into
curl:masterfrom
jay:hdrparse

Conversation

@jay

@jay jay commented Nov 14, 2023

Copy link
Copy Markdown
Member
  • Don't dereference the past-the-end element when parsing the server's Content-disposition header.

As 'p' is advanced it can point to the past-the-end element and prior to this change 'p' could be dereferenced in that case.

Technically the past-the-end element is not out of bounds because dynbuf (which manages the header line) automatically adds a null terminator to every buffer and that is not included in the buffer length passed to the header callback.

Closes #xxxxx


to force past-the-end dereference set server content-disposition header to max length (102399).

while true; do { perl -e 'print ("HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: Close\r\nContent-Disposition:");print("A"x102378);print("\n\r\n")'; sleep 2; } | nc -4l 127.0.0.1 8000; done
curl -v -OJ http://localhost:8000/foo

note 102399 comes from CURL_MAX_HTTP_HEADER which is 102400 minus the 1 byte dynbuf uses at the end of every buffer to null terminate. if the server sent a length of 102400 for the header then dynbuf returns out of memory. due to this an access violation is not possible.

- Don't dereference the past-the-end element when parsing the server's
  Content-disposition header.

As 'p' is advanced it can point to the past-the-end element and prior
to this change 'p' could be dereferenced in that case.

Technically the past-the-end element is not out of bounds because dynbuf
(which manages the header line) automatically adds a null terminator to
every buffer and that is not included in the buffer length passed to
the header callback.

Closes #xxxxx
@jay jay closed this in efbbbf4 Nov 14, 2023
@jay jay deleted the hdrparse branch November 14, 2023 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants