Skip to content

Commit

Permalink
Fix pointer overflow in STRING_CAT
Browse files Browse the repository at this point in the history
The end pointer is positioned one past the end of the destination, and
it is undefined behavior to compute an address beyond the end pointer,
including for comparisons, even temporarily. The UB occurs exactly when
buffer overflow would have occurred, so the buffer overflow check could
be optimized away by compilers. Even if this wasn't the case, the check
could produce a false negative if the computed address overflowed the
address space, which is, after all, why the C standard doesn't define
behavior in the first place.

The fix is simple: Check using sizes, not addresses. The explicit cast
suppresses warnings about signed-unsigned comparisons, and the assertion
checks the cast.
  • Loading branch information
skeeto committed Feb 18, 2024
1 parent a8dd531 commit 3f93667
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/lib/ec_glob.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include "global.h"

#include <assert.h>
#include <ctype.h>
#include <string.h>
#include <pcre2.h>
Expand All @@ -51,7 +52,8 @@ static const UT_icd ut_int_pair_icd = {sizeof(int_pair),NULL,NULL,NULL};
/* concatenate the string then move the pointer to the end */
#define STRING_CAT(p, string, end) do { \
size_t string_len = strlen(string); \
if (p + string_len >= end) \
assert(end > p); \
if (string_len >= (size_t)(end - p)) \
return -1; \
strcat(p, string); \
p += string_len; \
Expand Down

0 comments on commit 3f93667

Please sign in to comment.