Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CVE-2018-7039] buffer overflow in ccnl_ndntlv_prependBlob #191

Open
mfrey opened this issue Feb 13, 2018 · 2 comments
Open

[CVE-2018-7039] buffer overflow in ccnl_ndntlv_prependBlob #191

mfrey opened this issue Feb 13, 2018 · 2 comments

Comments

@mfrey
Copy link
Collaborator

mfrey commented Feb 13, 2018

Hi,

I think that there are multiple issues with various ccnl_<packetformat>_prependBlob functions. I've picked the ccnl_ndntlv_prependBlob as an example.

The len parameter refers to the size of the blob and offset to the position where the data in blob should be written to buf. The function returns -1 if the offset is lower than the size of the blob which is prepended to the buffer (basically if somebody tries to write before the buffer (buffer underwrite)).

445 int
446 ccnl_ndntlv_prependBlob(int type, unsigned char *blob, int len,
447                         int *offset, unsigned char *buf)
448 {
449     int oldoffset = *offset;
450 
451     if (*offset < len)
452         return -1;
453     memcpy(buf + *offset - len, blob, len);

I've made a short illustration depicting valid input parameters and the result after the memcpy operation.

image

Do you agree? Also, the function lacks the size of the buffer which would allow a better error handling/invalid parameters.

TIA
Michael

@mfrey
Copy link
Collaborator Author

mfrey commented Feb 13, 2018

After a brief discussion we agreed that this not vulnerability but a bug. As far as I can see throughout the code base ccnl_ndntlv_prependBlob is used with safe parameter settings, but we should fix the function for future developers/users.

@blacksheeep
Copy link
Contributor

blacksheeep commented Feb 13, 2018

After additional discussion, we have to add a vulnerability tag again.
This is a remote vulnerability. By adding incorrect TLV information, it is possible that the length of a component inside prefix does not correspond to the allocated data.
Therefore, name->complen[cnt] can be < 0, since the length field of a NDNTLV can have up to 64bit, while name->complen[cnt] is only a 32bit integer.

508        if (ccnl_ndntlv_prependBlob(NDN_TLV_NameComponent, name->comp[cnt],
509           name->complen[cnt], offset, buf) < 0)

To fix that, we have to change the type of complen to uint64_t inside ccnl_prefix_s.

@mfrey mfrey changed the title buffer overflow in ccnl_ndntlv_prependBlob [CVE-2018-7039] buffer overflow in ccnl_ndntlv_prependBlob Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants