From 394b48b9f5ff44497da03921cb871d1232478b99 Mon Sep 17 00:00:00 2001 From: Dmitry Verenitsin Date: Sat, 8 Aug 2026 23:40:30 +0500 Subject: [PATCH] [core] Harden switch_b64_decode output bound and input handling (#3110) switch_b64_decode bounded its writes with `ol >= olen - 1`, where `olen` is unsigned. An `olen` of 0 made `olen - 1` wrap to `SIZE_MAX`, so the bound never fired and the loop wrote the entire decoded input plus a trailing NUL past the destination. Reject `olen == 0` and test `ol + 1 >= olen` before each write, so the comparison never subtracts from an unsigned and always leaves room for the terminator. The alphabet lookup table `l64` was a `char` indexed by a `char`, which is unsafe whichever way `char` is signed: - Where `char` is signed, an input byte >= 0x80 became a negative index and read before the table. - Where `char` is unsigned, the `-1` "not in alphabet" sentinel was stored as 255, so the skip test never matched and non-alphabet bytes were folded in as data. Make `l64` a `signed char` and index it with `(unsigned char)`, so the sentinel survives and the index stays in range on every platform. Change the bit accumulator `b` from `int` to `unsigned int` to avoid signed-overflow undefined behavior on long input; decoded output is unchanged. Add unit tests: an encode/decode round-trip across the padding cases, the output-bound edges (`olen` of 0, 1, and a truncating buffer), and a non-alphabet byte (including one >= 0x80) that must be skipped. --- src/switch_utils.c | 17 +++++--- tests/unit/switch_utils.c | 87 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 6 deletions(-) diff --git a/src/switch_utils.c b/src/switch_utils.c index 854659ab4a..ad0f96b70c 100644 --- a/src/switch_utils.c +++ b/src/switch_utils.c @@ -1062,22 +1062,27 @@ SWITCH_DECLARE(switch_status_t) switch_b64_encode(unsigned char *in, switch_size SWITCH_DECLARE(switch_size_t) switch_b64_decode(const char *in, char *out, switch_size_t olen) { - char l64[256]; - int b = 0, c, l = 0, i; + signed char l64[256]; + int c, l = 0, i; + unsigned int b = 0; const char *ip; char *op = out; size_t ol = 0; + if (olen == 0) { /* no room even for the trailing NUL */ + return 0; + } + for (i = 0; i < 256; i++) { l64[i] = -1; } for (i = 0; i < 64; i++) { - l64[(int) switch_b64_table[i]] = (char) i; + l64[(unsigned char) switch_b64_table[i]] = (signed char) i; } for (ip = in; ip && *ip && (*ip != '='); ip++) { - c = l64[(int) *ip]; + c = l64[(unsigned char) *ip]; if (c == -1) { continue; } @@ -1086,10 +1091,10 @@ SWITCH_DECLARE(switch_size_t) switch_b64_decode(const char *in, char *out, switc l += 6; while (l >= 8) { - op[ol++] = (char) ((b >> (l -= 8)) % 256); - if (ol >= olen - 1) { + if (ol + 1 >= olen) { /* reserve the last byte for the NUL */ goto end; } + op[ol++] = (char) ((b >> (l -= 8)) % 256); } } diff --git a/tests/unit/switch_utils.c b/tests/unit/switch_utils.c index 959f9f2e4f..a883c4602c 100644 --- a/tests/unit/switch_utils.c +++ b/tests/unit/switch_utils.c @@ -124,6 +124,93 @@ FST_TEST_BEGIN(b64_pad1) } FST_TEST_END() +FST_TEST_BEGIN(b64_roundtrip) +{ + /* Encode then decode inputs covering all three padding cases; the base64 output must + match the known value and decode back to the original bytes. Unlike b64_pad1/b64_pad2 + (all-zero input), these push non-zero bytes through the padded final group. */ + struct { + const char *plain; + const char *encoded; + } cases[] = { + { "Man", "TWFu" }, /* no padding */ + { "Ma", "TWE=" }, /* one pad byte */ + { "M", "TQ==" }, /* two pad bytes */ + { "Hello, World!", "SGVsbG8sIFdvcmxkIQ==" } + }; + int i; + + for (i = 0; i < (int) (sizeof(cases) / sizeof(cases[0])); i++) { + unsigned char encoded[64]; + char decoded[64]; + switch_size_t plain_len = strlen(cases[i].plain); + switch_size_t decoded_len; + switch_status_t status = switch_b64_encode((unsigned char *) cases[i].plain, plain_len, encoded, sizeof(encoded)); + + fst_xcheck(status == SWITCH_STATUS_SUCCESS, "encode must succeed"); + fst_check_string_equals((const char *) encoded, cases[i].encoded); + + decoded_len = switch_b64_decode((const char *) encoded, decoded, sizeof(decoded)); + fst_xcheck(decoded_len == plain_len + 1, "decode must return the plaintext length plus the trailing NUL"); + fst_check_string_equals(decoded, cases[i].plain); + } +} +FST_TEST_END() + +FST_TEST_BEGIN(b64_decode_output_bounds) +{ + /* The 0xAA sentinel across the destination catches any write outside the region + the decode call is allowed to touch. */ + unsigned char guarded[32]; + switch_size_t size; + int i; + + /* Decode with olen == 0: no room even for the trailing NUL, so the decoder must + write nothing and return 0. */ + memset(guarded, 0xAA, sizeof(guarded)); + size = switch_b64_decode("QUJDQUJDQUJDQUJDQUJDQUJDQUJDQUJD", (char *) guarded, 0); + fst_xcheck(size == 0, "olen==0 decode must return 0"); + for (i = 0; i < (int) sizeof(guarded); i++) { + fst_xcheck(guarded[i] == 0xAA, "olen==0 decode must not write any output byte"); + } + + /* Decode with olen == 1: room only for the terminating NUL at index 0; no decoded + data byte may be written. */ + memset(guarded, 0xAA, sizeof(guarded)); + size = switch_b64_decode("QUJDQUJDQUJDQUJD", (char *) guarded, 1); + fst_xcheck(size == 1, "olen==1 decode must return 1 (NUL only)"); + fst_xcheck(guarded[0] == '\0', "olen==1 decode must store the NUL at index 0"); + for (i = 1; i < (int) sizeof(guarded); i++) { + fst_xcheck(guarded[i] == 0xAA, "olen==1 decode must not write past index 0"); + } + + /* Decode with a small olen: up to olen-1 decoded bytes, then the trailing NUL at + index olen-1, and nothing beyond. "QUJD" decodes to "ABC". */ + memset(guarded, 0xAA, sizeof(guarded)); + size = switch_b64_decode("QUJD", (char *) guarded, 2); + fst_xcheck(size == 2, "bounded decode must return olen"); + fst_xcheck(guarded[0] == 'A', "first decoded byte must be written"); + fst_xcheck(guarded[1] == '\0', "trailing NUL must be at index olen-1"); + for (i = 2; i < (int) sizeof(guarded); i++) { + fst_xcheck(guarded[i] == 0xAA, "bounded decode must not write past index olen-1"); + } +} +FST_TEST_END() + +FST_TEST_BEGIN(b64_decode_non_alphabet_bytes) +{ + /* Bytes outside the base64 alphabet, including those >= 0x80, are skipped and never used + as a lookup-table index. */ + char decoded[8]; + switch_size_t size; + + /* "QUJD" ("ABC") with a non-alphabet 0x80 byte spliced in. */ + size = switch_b64_decode("QU\x80" "JD", decoded, sizeof(decoded)); + fst_xcheck(size == 4, "non-alphabet byte must be skipped, leaving 3 data bytes plus the NUL"); + fst_check_string_equals(decoded, "ABC"); +} +FST_TEST_END() + #define test_uri_count 6 /* Currently tests only clear_uri() */