Skip to content
This repository has been archived by the owner on Jun 30, 2021. It is now read-only.

The function htparser_run may have a out of bounds memory write #69

Closed
linimbus opened this issue Jan 3, 2018 · 3 comments
Closed

The function htparser_run may have a out of bounds memory write #69

linimbus opened this issue Jan 3, 2018 · 3 comments
Assignees

Comments

@linimbus
Copy link

linimbus commented Jan 3, 2018

Although there had judged upper limit
https://github.com/criticalstack/libevhtp/blob/bc52552641ef9d01713c6d8e9b41aa1c089091fe/parser.c#L728

But, if p->buf_idx arrival at (PARSER_STACK_MAX-1) value, The following will using two characters of space, then have a out of bounds memory write.
https://github.com/criticalstack/libevhtp/blob/bc52552641ef9d01713c6d8e9b41aa1c089091fe/parser.c#L776-L777

Proposed using if (p->buf_idx >= PARSER_STACK_MAX-1) replace.

@NathanFrench
Copy link
Collaborator

Hey Nathan, it's 1997 calling!

I'll fix this up shortly. Thank you very much!

NathanFrench added a commit that referenced this issue Jan 3, 2018
As reported via #69,
there is a slight chance that 1 byte can overflow parser->buf[STACK_MAX]
when adding the extra '\0'

Added the macro `HTP_SET_BUF(CH)` which does proper bounds checks before
writing the final '\0'.
@NathanFrench
Copy link
Collaborator

Can you check out https://github.com/criticalstack/libevhtp/compare/issue_69 and see if this seems resolved to you?

@NathanFrench
Copy link
Collaborator

I ran through a bunch of tests, and this macro doesn't seem to change anything other than the 1 byte check. So I'm going to merge and close this out.

Thanks mucho!

NathanFrench added a commit that referenced this issue Jan 4, 2018
[#69] Fix potential out of bound write to p->buf
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants