fix: resolve UBSAN violations in the codebase

* make EXPECT_BYTEARRAY_EQUAL to do memcmp when length input isn't zero * proceed regardless of inputs if length is set to be zero * set results of null arithmetic to null pointer instead of undefine
boquan-fang · Aug 21, 2024 · 7f9fda1 · 7f9fda1
1 parent 057e2ca
commit 7f9fda1
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 4 deletions.
diff --git a/stuffer/s2n_stuffer.c b/stuffer/s2n_stuffer.c
@@ -327,7 +327,7 @@ int s2n_stuffer_write_bytes(struct s2n_stuffer *stuffer, const uint8_t *data, co
     POSIX_PRECONDITION(s2n_stuffer_validate(stuffer));
     POSIX_GUARD(s2n_stuffer_skip_write(stuffer, size));
 
-    void *ptr = stuffer->blob.data + stuffer->write_cursor - size;
+    void *ptr = (stuffer->blob.data == NULL) ? NULL : stuffer->blob.data + stuffer->write_cursor - size;
     POSIX_ENSURE(S2N_MEM_IS_READABLE(ptr, size), S2N_ERR_NULL);
 
     if (ptr == data) {
@@ -437,7 +437,7 @@ int s2n_stuffer_shift(struct s2n_stuffer *stuffer)
     POSIX_ENSURE_REF(stuffer);
     struct s2n_stuffer copy = *stuffer;
     POSIX_GUARD(s2n_stuffer_rewrite(&copy));
-    uint8_t *data = stuffer->blob.data + stuffer->read_cursor;
+    uint8_t *data = (stuffer->blob.data == NULL) ? NULL : stuffer->blob.data + stuffer->read_cursor;
     uint32_t data_size = s2n_stuffer_data_available(stuffer);
     POSIX_GUARD(s2n_stuffer_write_bytes(&copy, data, data_size));
     *stuffer = copy;

diff --git a/tests/s2n_test.h b/tests/s2n_test.h
@@ -200,7 +200,15 @@ int test_count;
 /* for use with S2N_RESULT */
 #define EXPECT_OK( function_call )  EXPECT_TRUE( s2n_result_is_ok(function_call) )
 
-#define EXPECT_BYTEARRAY_EQUAL( p1, p2, l ) EXPECT_EQUAL( memcmp( (p1), (p2), (l) ), 0 )
+#define EXPECT_BYTEARRAY_EQUAL( p1, p2, l ) \
+    do { \
+        if (l != 0) { \
+            EXPECT_EQUAL( memcmp( (p1), (p2), (l) ), 0 ); \
+        } else { \
+            EXPECT_EQUAL(0, 0); \
+        } \
+    } while (0)
+
 #define EXPECT_BYTEARRAY_NOT_EQUAL( p1, p2, l ) EXPECT_NOT_EQUAL( memcmp( (p1), (p2), (l) ), 0 )
 
 #define EXPECT_STRING_EQUAL( p1, p2 ) EXPECT_EQUAL( strcmp( (p1), (p2) ), 0 )

diff --git a/tests/testlib/s2n_test_server_client.c b/tests/testlib/s2n_test_server_client.c
@@ -78,7 +78,7 @@ S2N_RESULT s2n_negotiate_test_server_and_client_with_early_data(struct s2n_conne
     do {
         if (!client_early_done) {
             bool success = s2n_send_early_data(client_conn,
-                                   early_data_to_send->data + total_data_sent,
+                                   (early_data_to_send->data == NULL) ? NULL : early_data_to_send->data + total_data_sent,
                                    early_data_to_send->size - total_data_sent,
                                    &data_sent, &blocked)
                     == S2N_SUCCESS;