-
Notifications
You must be signed in to change notification settings - Fork 35
Fix problem exactly filling a buffer when encoding. #49
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,7 +35,7 @@ typedef struct _write_state | |
ssize_t size; | ||
} cn_write_state; | ||
|
||
#define ensure_writable(sz) if ((ws->offset<0) || (ws->offset + (sz) >= ws->size)) { \ | ||
#define ensure_writable(sz) if ((ws->offset<0) || (ws->size - ws->offset < (sz))) { \ | ||
ws->offset = -1; \ | ||
return; \ | ||
} | ||
|
@@ -302,6 +302,7 @@ ssize_t cn_cbor_encoder_write(uint8_t *buf, | |
const cn_cbor *cb) | ||
{ | ||
cn_write_state ws = { buf, buf_offset, buf_size }; | ||
if (ws.size < 0) { return -1; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When would this be the case? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In normal use I would expect it would not happen, but if buf_size has the high bit set and buf_offset is non-zero the check in ensure_writable could underflow. Probably an overabundance of caution, but issue #12 made me think about what could happen in an attack situation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you like to see this removed? Or maybe change ws.size from ssize_t to size_t? I'd like to see the off by one error fixed. It seems there were attempts to do so since 2015 that never quite made it. |
||
_visit(cb, _encoder_visitor, _encoder_breaker, &ws); | ||
if (ws.offset < 0) { return -1; } | ||
return ws.offset - buf_offset; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely a good change...