in-place decode and regular decode return BAD_PAYLOAD if there are an…

…y non-terminal zero bytes in the payload (#44) * in-place decode and regular decode return BAD_PAYLOAD if there are any non-terminal zero bytes in the payload * actions v3, MP on win
charlesnicholson · Dec 21, 2022 · 9482051 · 9482051
1 parent dbdc6e9
commit 9482051
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 20 deletions.
diff --git a/.github/workflows/presubmit.yml b/.github/workflows/presubmit.yml
@@ -25,7 +25,7 @@ jobs:
       CXX: /usr/bin/clang++
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - name: Build
         run: COBS_SANITIZE=1 make -j
 
@@ -46,15 +46,15 @@ jobs:
         architecture: [32, 64]
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - name: Build
         run: COBS_LINUXARCH=${{ matrix.architecture }} make -j
 
   macos:
     runs-on: macos-latest
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - name: Build
         run: make -j
 
@@ -66,7 +66,7 @@ jobs:
         architecture: [32, 64]
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - name: Build
         shell: cmd
         run: |

diff --git a/cobs.c b/cobs.c
@@ -4,7 +4,6 @@
 
 typedef unsigned char cobs_byte_t;
 
-
 cobs_ret_t cobs_encode_inplace(void *buf, unsigned len) {
   if (!buf || (len < 2)) { return COBS_RET_ERR_BAD_ARG; }
 
@@ -30,14 +29,16 @@ cobs_ret_t cobs_encode_inplace(void *buf, unsigned len) {
   return COBS_RET_SUCCESS;
 }
 
-
 cobs_ret_t cobs_decode_inplace(void *buf, unsigned const len) {
   if (!buf || (len < 2)) { return COBS_RET_ERR_BAD_ARG; }
 
   cobs_byte_t *const src = (cobs_byte_t *)buf;
   unsigned ofs, cur = 0;
   while (cur < len && ((ofs = src[cur]) != COBS_FRAME_DELIMITER)) {
     src[cur] = 0;
+    for (unsigned i = 1; i < ofs; ++i) {
+      if (src[cur + i] == 0) { return COBS_RET_ERR_BAD_PAYLOAD; }
+    }
     cur += ofs;
   }
 
@@ -47,7 +48,6 @@ cobs_ret_t cobs_decode_inplace(void *buf, unsigned const len) {
   return COBS_RET_SUCCESS;
 }
 
-
 cobs_ret_t cobs_encode(void const *dec,
                        unsigned dec_len,
                        void *out_enc,
@@ -65,7 +65,6 @@ cobs_ret_t cobs_encode(void const *dec,
   return r;
 }
 
-
 cobs_ret_t cobs_encode_inc_begin(void *out_enc,
                                  unsigned enc_max,
                                  cobs_enc_ctx_t *out_ctx) {
@@ -81,7 +80,6 @@ cobs_ret_t cobs_encode_inc_begin(void *out_enc,
   return COBS_RET_SUCCESS;
 }
 
-
 cobs_ret_t cobs_encode_inc(cobs_enc_ctx_t *ctx,
                            void const *dec,
                            unsigned dec_len) {
@@ -133,7 +131,6 @@ cobs_ret_t cobs_encode_inc(cobs_enc_ctx_t *ctx,
   return COBS_RET_SUCCESS;
 }
 
-
 cobs_ret_t cobs_encode_inc_end(cobs_enc_ctx_t *ctx, unsigned *out_enc_len) {
   if (!ctx || !out_enc_len) { return COBS_RET_ERR_BAD_ARG; }
 
@@ -145,7 +142,6 @@ cobs_ret_t cobs_encode_inc_end(cobs_enc_ctx_t *ctx, unsigned *out_enc_len) {
   return COBS_RET_SUCCESS;
 }
 
-
 cobs_ret_t cobs_decode(void const *enc,
                        unsigned enc_len,
                        void *out_dec,
@@ -170,6 +166,7 @@ cobs_ret_t cobs_decode(void const *enc,
 
     if ((dst_idx + code - 1) > dec_max) { return COBS_RET_ERR_EXHAUSTED; }
     for (unsigned i = 0; i < code - 1; ++i) {
+      if (src[src_idx] == 0) { return COBS_RET_ERR_BAD_PAYLOAD; }
       dst[dst_idx++] = src[src_idx++];
     }
 

diff --git a/make-win.bat b/make-win.bat
@@ -1,4 +1,4 @@
-cl.exe /W4 /WX /EHsc ^
+cl.exe /W4 /WX /MP /EHsc ^
     cobs.c ^
     tests/test_cobs_decode.cc ^
     tests/test_cobs_decode_inplace.cc ^

diff --git a/tests/test_cobs_decode.cc b/tests/test_cobs_decode.cc
@@ -1,16 +1,26 @@
 #include "../cobs.h"
+#include "byte_vec.h"
 #include "doctest.h"
 
-#include <cstring>
-
 TEST_CASE("Decoding validation") {
-  unsigned char enc[32], dec[32];
+  unsigned char dec[32];
   unsigned dec_len;
 
-  SUBCASE("Invalid payload") {
-    // first byte jumps past end
-    enc[0] = 3;
-    enc[1] = 0;
-    REQUIRE(cobs_decode(&enc, 2, dec, sizeof(enc), &dec_len) == COBS_RET_ERR_BAD_PAYLOAD);
+  SUBCASE("Invalid payload: jump past end") {
+    byte_vec_t enc{3, 0};
+    REQUIRE(cobs_decode(enc.data(),
+                        unsigned(enc.size()),
+                        dec,
+                        sizeof(dec),
+                        &dec_len) == COBS_RET_ERR_BAD_PAYLOAD);
+  }
+
+  SUBCASE("Invalid payload: jump over internal zeroes") {
+    byte_vec_t enc{5, 1, 0, 0, 1, 0};
+    REQUIRE(cobs_decode(enc.data(),
+                        unsigned(enc.size()),
+                        dec,
+                        sizeof(dec),
+                        &dec_len) == COBS_RET_ERR_BAD_PAYLOAD);
   }
 }
diff --git a/tests/test_cobs_decode_inplace.cc b/tests/test_cobs_decode_inplace.cc
@@ -36,6 +36,10 @@ TEST_CASE("Inplace decoding validation") {
     REQUIRE(cobs_decode_vec(buf) == COBS_RET_ERR_BAD_PAYLOAD);
     buf = byte_vec_t{0x01, 0x00, 0x00}; // land on an interior 0x00
     REQUIRE(cobs_decode_vec(buf) == COBS_RET_ERR_BAD_PAYLOAD);
+    buf = byte_vec_t{0x02, 0x00, 0x00}; // jump over an interior 0x00
+    REQUIRE(cobs_decode_vec(buf) == COBS_RET_ERR_BAD_PAYLOAD);
+    buf = byte_vec_t{0x04, 0x01, 0x00, 0x01, 0x00}; // jump over interior 0x00
+    REQUIRE(cobs_decode_vec(buf) == COBS_RET_ERR_BAD_PAYLOAD);
   }
 }