Project

General

Profile

Actions

Security #6902

closed

base64: off-by-three overflow in DecodeBase64()

Added by Victor Julien 7 months ago. Updated 6 months ago.

Status:
Closed
Priority:
Normal
Target version:
Affected Versions:
Label:
Git IDs:

fd47e67dc65f9111895c88fb406c938b1f857325

Severity:
HIGH
Disclosure Date:

Description

static inline void DecodeBase64Block(uint8_t ascii[ASCII_BLOCK], uint8_t b64[B64_BLOCK])
{
    ascii[0] = (uint8_t) (b64[0] << 2) | (b64[1] >> 4);
    ascii[1] = (uint8_t) (b64[1] << 4) | (b64[2] >> 2);
    ascii[2] = (uint8_t) (b64[2] << 6) | (b64[3]);
}

Base64Ecode DecodeBase64(uint8_t *dest, uint32_t dest_size, const uint8_t *src, uint32_t len,
        uint32_t *consumed_bytes, uint32_t *decoded_bytes, Base64Mode mode)
{
    int val;
    uint32_t padding = 0, bbidx = 0, sp = 0, leading_sp = 0;
    uint8_t *dptr = dest;
    uint8_t b64[B64_BLOCK] = { 0,0,0,0 };
    bool valid = true;
    Base64Ecode ecode = BASE64_ECODE_OK;

    *decoded_bytes = 0;

    for (uint32_t i = 0; i < len; i++) {
        val = GetBase64Value(src[i]);
        if (val < 0) {
            if (mode == BASE64_MODE_RFC2045 && src[i] != '=') {
                if (bbidx == 0) {
                    leading_sp++;
                }
                sp++;
                continue;
            }
            if (src[i] != '=') {
                valid = false;
                ecode = BASE64_ECODE_ERR;
                if (mode == BASE64_MODE_STRICT) {
                    *decoded_bytes = 0;
                }
                break;
            }
            padding++;
        }

        b64[bbidx++] = (val > 0 ? (uint8_t)val : 0);

        if (bbidx == B64_BLOCK) {
[1]         uint32_t numDecoded_blk = ASCII_BLOCK - (padding < B64_BLOCK ? padding : ASCII_BLOCK);
[2]         if (dest_size < *decoded_bytes + numDecoded_blk) {
                SCLogDebug("Destination buffer full");
                ecode = BASE64_ECODE_BUF;
                break;
            }
[3]         DecodeBase64Block(dptr, b64);
            dptr += numDecoded_blk;
            *decoded_bytes += numDecoded_blk;
            bbidx = 0;
            padding = 0;
            *consumed_bytes += B64_BLOCK + sp;
            sp = 0;
            leading_sp = 0;
            memset(&b64, 0, sizeof(b64));
        }
    }

    ...

    if (valid && bbidx > 0 && (mode != BASE64_MODE_RFC2045)) {
        *decoded_bytes += ASCII_BLOCK - (B64_BLOCK - bbidx);
[4]     DecodeBase64Block(dptr, b64);
    }

    ..
}

Consider the situation when destination buffer is full, ie 'dest_size' is equal to '*decoded_bytes'.
1. - in case 'padding' is equal to B64_BLOCK (4), 'numDecoded_blk' will be set to 0
2. - we will pass this check, as 'dest_size' is equal to '*decoded_bytes'
3. - 3 bytes will be written past the end of buffer 'dptr'
4. - similar issue

How to verify:
1) get source code
$ git clone https://github.com/OISF/suricata

2) Edit src/util-base64.c and change B64DecodeCompleteString() test.
Change string "SGVsbG8gV29ybGR6" to "SGVsbG8gV29ybGR6===="

3) build
$ export CFLAGS="-ggdb3 -Werror -Wchar-subscripts -fno-strict-aliasing -fstack-protector-all -fsanitize=address -fno-omit-frame-pointer -Wno-unused-parameter -Wno-unused-function"
$ ./autoconf.sh
$ ./configure --enable-unittests
$ make

4) run
$ ./src/suricata -U B64DecodeCompleteString -u

ASAN LOG:

==77257==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffcc360882c at pc 0x55907c573699 bp 0x7ffcc36086c0 sp 0x7ffcc36086b0
WRITE of size 1 at 0x7ffcc360882c thread T0 (Suricata-Main)
    #0 0x55907c573698 in DecodeBase64Block suricata/src/util-base64.c:91
    #1 0x55907c573c75 in DecodeBase64 suricata/src/util-base64.c:161
    #2 0x55907c574378 in B64DecodeCompleteString suricata/src/util-base64.c:241
    #3 0x55907bc7a22c in UtRunTests suricata/src/util-unittest.c:212
    #4 0x55907c43fffb in RunUnittests suricata/src/runmode-unittests.c:286
    #5 0x55907bbd6460 in StartInternalRunMode suricata/src/suricata.c:2335
    #6 0x55907bbd8fc6 in SuricataMain suricata/src/suricata.c:2901
    #7 0x55907bbc98eb in main suricata/src/main.c:22
    #8 0x7f3b31a29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #9 0x7f3b31a29e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #10 0x55907bbc97f4 in _start (suricata/src/.libs/suricata+0x3107f4)

Subtasks 2 (0 open2 closed)

Security #6905: base64: off-by-three overflow in DecodeBase64() (7.0.x backport)ClosedPhilippe AntoineActions
Security #6931: base64: off-by-three overflow in DecodeBase64() (6.0.x backport)ClosedPhilippe AntoineActions
Actions #1

Updated by Philippe Antoine 7 months ago

Looks like the third call to DecodeBase64Block has the right bounds check

            if (dest_size - *decoded_bytes < 3)
                return BASE64_ECODE_BUF;

Actions #2

Updated by Philippe Antoine 7 months ago

  • Status changed from New to In Review
  • Label Needs backport to 7.0 added

Gitlab MR

Actions #3

Updated by Philippe Antoine 7 months ago

Assessment :
can be reached from:
- datasets loading
- from base64_decode but only if all the rules using this keyword use bytes option with values 1, 2 or 5 !
- from smtp/mime traffic, but as it can only overwrite 3 bytes of data (and only with value 0), that is only md5_ctx pointer that can get changed, so it affects only if you have the non-default app-layer.protocols.smtp.mime.body-md5 config option set to yes

So, I would say severity is high

Actions #4

Updated by Shivani Bhardwaj 7 months ago

  • Assignee changed from Shivani Bhardwaj to Philippe Antoine
  • Target version changed from TBD to 8.0.0-beta1
Actions #5

Updated by OISF Ticketbot 7 months ago

  • Subtask #6905 added
Actions #6

Updated by OISF Ticketbot 7 months ago

  • Label deleted (Needs backport to 7.0)
Actions #7

Updated by Philippe Antoine 7 months ago

Just some quick note about why this was not found by fuzzing :
- there is no fuzz targets for datasets loading...
- keyword base64_decode does not look fuzzing-friendly as it is not stateless : the size of the buffer is a global maxed one, instead of one per rule
- hard to reach from SMTP, because you need exactly 3072 (big number) bytes of base64 decoded without problem, then your evil pattern

Actions #9

Updated by Victor Julien 7 months ago

  • Severity changed from MODERATE to HIGH
Actions #10

Updated by Victor Julien 7 months ago

  • Label Needs backport to 6.0 added
Actions #11

Updated by OISF Ticketbot 7 months ago

  • Subtask #6931 added
Actions #12

Updated by OISF Ticketbot 7 months ago

  • Label deleted (Needs backport to 6.0)
Actions #13

Updated by Victor Julien 6 months ago

  • CVE set to 2024-32664
Actions #14

Updated by Victor Julien 6 months ago

  • Status changed from In Review to Closed
  • Git IDs updated (diff)
Actions

Also available in: Atom PDF