Fix buffer overrun found with address sanitizers

[mreed348]

See commit messages for more info. And thank you for this project!

[mreed348]

Here's another (IMO cleaner) way to fix the problem:

diff --git a/cobs.c b/cobs.c
index c080867..1e0e324 100644
--- a/cobs.c
+++ b/cobs.c
@@ -36,10 +36,9 @@ cobs_ret_t cobs_decode_inplace(void *buf, unsigned const len) {
 
   cobs_byte_t *const src = (cobs_byte_t *)buf;
   unsigned ofs, cur = 0;
-  while ((ofs = src[cur]) != COBS_FRAME_DELIMITER) {
+  while (cur < len && ((ofs = src[cur]) != COBS_FRAME_DELIMITER)) {
     src[cur] = 0;
     cur += ofs;
-    if (cur >= len) { return COBS_RET_ERR_BAD_PAYLOAD; }
   }
 
   if (cur != len - 1) { return COBS_RET_ERR_BAD_PAYLOAD; }

any preference?

[charlesnicholson]

How embarrassing :-D

[charlesnicholson]

Thanks a ton for the PR; this is what happens when I don't UBSAN / ASAN from day 1, sigh :)

I agree, I like your "cleaner" version more as well, though I'd happily merge either, lemme know. Do you happen to have a test case? It'd be nice to harness this w/a unit test as well as the sanitizer.

Also if you want to add a CONTRIBUTORS.md file to the root and throw your name at the top, feel free! (You could copy https://github.com/charlesnicholson/nanoprintf/blob/main/CONTRIBUTORS.md as a template if you want).

[mreed348]

I agree, I like your "cleaner" version more as well, though I'd happily merge either, lemme know.

Sounds good, just amended the commit with the "cleaner" version.

Do you happen to have a test case? It'd be nice to harness this w/a unit test as well as the sanitizer.

Ah, I forgot to mention that this already has a test case, which failed when I turned sanitizers on (but before I fixed the bug in cobs_decode_inplace): https://github.com/charlesnicholson/nanocobs/blob/main/tests/test_cobs_decode_inplace.cc#L31-L32

Also if you want to add a CONTRIBUTORS.md file to the root and throw your name at the top, feel free! (You could copy https://github.com/charlesnicholson/nanoprintf/blob/main/CONTRIBUTORS.md as a template if you want).

Done!

[charlesnicholson]

Thanks for this! I'll fix the issue you filed and then cut a new release, unless turning on the sanitizers reveals any other horrors :)