darconeous / libnyoci

A flexible CoAP stack for embedded devices and computers. RFC7252 compatible.
Other
27 stars 10 forks source link

buffer overrun in nyoci_outbound_set_uri() #10

Closed snej closed 6 years ago

snej commented 6 years ago

The uri_copy string, and the components that point into it, can get corrupted in nyoci_outbound_set_uri() when HAVE_ALLOCA is false and NYOCI_AVOID_MALLOC is true. In this configuration the function steals space from the packet buffer to copy the uri into, but it misjudges how much of the buffer will be used by options:

I'm working with a case where the uri string is coap://192.168.1.64:12345/db. By the time components.host is accessed on line 502, uri_copy has been partially overwritten and components.host is 9?2.168.1.64. This means the hostname looked up is wrong and the request fails.

darconeous commented 6 years ago

Will have a quick look.

darconeous commented 6 years ago

Indeed, the existing code is questionable. You can likely work-around your problem temporarily by changing the references to 8 to something bigger like 32 or 64. I'll come up with a permanent fix, unless you happen to have one on hand.

darconeous commented 6 years ago

I wonder if one of the problems is that coap_encode_option() is using memcpy() instead of memmove(). That seems unfortunate, but easy to fix.

darconeous commented 6 years ago

Mind checking if change ed72535 from this branch addresses your problems?

snej commented 6 years ago

Yes, that patch fixes my problem. Thanks!