Skip to content

Conversation

@MegaManSec
Copy link
Contributor

The loop hex-encodes 16 random bytes into a 32-byte cookie but used snprintf directly into the output buffer s, writing 3 bytes per iteration (2 hex + NUL) and overflowing by 1 byte on the last write. Hex-encode into a small temp and memcpy 2 bytes per iteration so exactly cookie_len bytes are written with no trailing NUL.

The loop hex-encodes 16 random bytes into a 32-byte cookie but used
snprintf directly into the output buffer s, writing 3 bytes per
iteration (2 hex + NUL) and overflowing by 1 byte on the last write.
Hex-encode into a small temp and memcpy 2 bytes per iteration so
exactly cookie_len bytes are written with no trailing NUL.
@willco007
Copy link
Member

There is a comment on line 1425 that says the null terminator is already being taken into account with the +1 on the size of buffer. How did you test this?

@MegaManSec
Copy link
Contributor Author

The +1 in the comment is about the temporary random-byte buffer, not the actual output field, s.

snprintf((char )&s[i2], 3, "%02X", buffer[i]);

So, on the final loop, we add an extra \0 to s past the buffer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants