heap buffer overflow read in coap_add_option function about libcoap

[mrdeep1]

@zambbo What is the value you are using for COAP_MAX_PDU_SIZE ? That #define disappeared back in 2017, and was replaced by COAP_DEFAULT_PDU_SIZE.

[zambbo]

oh, the #define COAP_MAX_PDU_SIZE 1152 is dropped. the value of COAP_MAX_PDU_SIZE is 1152

[mrdeep1]

OK. An issue is that coap_add_option(pdu, COAP_OPTION_URI_PORT, 8, data); is stating that the size of data is 8, when in reality it is 4.

[zambbo]

Yes, if the 3rd argument in coap_add_option function is greater than 4th argument buffer size, buffer overflow occurs in memcpy function in coap_opt_encode.

[mrdeep1]

OK, but is user error in what was passed to coap_add_option() that causes the overflow. coap_opt_encode() has no way of checking whether bad data is passed in or not.

[zambbo]

How about adding a length validation logic in coap_add_option to check for incorrect data being passed?

[mrdeep1]

Ignoring the requirement that the API does not change for now. I’m not sure how you can check the size of a variable within the function, so this would need to be passed in, but again there could be user error. In your case, the 4 bytes following data are undefined, hence your found issue and I could have expected you to have used sizeof(data). But that would fail if data was a pointer.

[mrdeep1]

Any application call to a public libcoap API that provides (and states) a buffer size bigger than the actual size of the buffer, then any fuzzing logic should pick this up (which may be in a called sub-function). It is not a libcoap issue unless libcoap itself is doing this from its own internal data management.

As this example is for bad application provided data + data_size, I am closing this Issue. If however you do find an internal libcoap issue, then please raise a new Issue with the details to be able to reproduce it.