Project

General

Profile

Actions

Bug #6903

closed

streaming buffer: heap overflows in StreamingBufferAppend()/StreamingBufferAppendNoTrack()

Added by Victor Julien 9 months ago. Updated 8 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

#define DATA_FITS(sb, len) ((sb)->region.buf_offset + (len) <= (sb)->region.buf_size)

int StreamingBufferAppend(StreamingBuffer *sb, const StreamingBufferConfig *cfg,
        StreamingBufferSegment *seg, const uint8_t *data, uint32_t data_len)
{
    DEBUG_VALIDATE_BUG_ON(seg == NULL);

    if (sb->region.buf == NULL) {
        if (InitBuffer(sb, cfg) == -1)
            return -1;
    }

[1]  if (!DATA_FITS(sb, data_len)) {
        if (sb->region.buf_size == 0) {
            if (GrowToSize(sb, cfg, data_len) != SC_OK)
                return -1;
        } else {
            if (GrowToSize(sb, cfg, sb->region.buf_offset + data_len) != SC_OK)
                return -1;
        }
    }

    DEBUG_VALIDATE_BUG_ON(!DATA_FITS(sb, data_len));

[2]  memcpy(sb->region.buf + sb->region.buf_offset, data, data_len);

1 - DATA_FITS() macro is vulnerable to integer overflow
2 - it will lead to heap overflow on this line

How to verify:

1) get source code
2) apply this patch:

--- current/src/util-streaming-buffer.c 2020-01-15 20:13:36.257117891 +0300
+++ suricata/src/util-streaming-buffer.c        2020-01-15 20:40:20.353179670 +0300
@@ -1836,7 +1836,14 @@
     StreamingBufferSegment seg1;
     FAIL_IF(StreamingBufferAppend(sb, &cfg, &seg1, (const uint8_t *)"ABCDEFGH", 8) != 0);
+    StreamingBufferSegment seg2;
+    unsigned int data_len = 0xffffffff;
+    unsigned char *ptr = malloc(data_len);
+    FAIL_IF(StreamingBufferAppend(sb, &cfg, &seg2, ptr, data_len) != 0);
+
+    return 0;
+   
     FAIL_IF(StreamingBufferAppend(sb, &cfg, &seg2, (const uint8_t *)"01234567", 8) != 0);
     FAIL_IF(sb->region.stream_offset != 0);
     FAIL_IF(sb->region.buf_offset != 16);

3) build as in previous issue

4) run unittest:

$ ./src/suricata -U StreamingBufferTest02 -u

ASAN LOG:

==77575==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6030000261b8 at pc 0x7f491f83a2c3 bp 0x7ffee377c170 sp 0x7ffee377b918
WRITE of size 4294967295 at 0x6030000261b8 thread T0 (Suricata-Main)
    #0 0x7f491f83a2c2 in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
    #1 0x5568fa174e86 in StreamingBufferAppend suricata/src/util-streaming-buffer.c:1090
    #2 0x5568fa17943a in StreamingBufferTest02 suricata/src/util-streaming-buffer.c:1843
    #3 0x5568f97e922c in UtRunTests suricata/src/util-unittest.c:212
    #4 0x5568f9faeffb in RunUnittests suricata/src/runmode-unittests.c:286
    #5 0x5568f9745460 in StartInternalRunMode suricata/src/suricata.c:2335
    #6 0x5568f9747fc6 in SuricataMain suricata/src/suricata.c:2901
    #7 0x5568f97388eb in main suricata/src/main.c:22
    #8 0x7f491f229d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #9 0x7f491f229e3f in __libc_start_main_impl ../csu/libc-start.c:392


Subtasks 1 (0 open1 closed)

Bug #6970: streaming buffer: heap overflows in StreamingBufferAppend()/StreamingBufferAppendNoTrack() (7.0.x backport)ClosedVictor JulienActions
Actions #1

Updated by Victor Julien 9 months ago

  • Description updated (diff)
Actions #2

Updated by Philippe Antoine 9 months ago

Is there some limit of 1Gbyte for streaming buffers ? This reminds me of something...

Actions #3

Updated by Victor Julien 9 months ago

Yes there is a per "region" max size of 1GiB. However, the issue happens before that is checked. The underflow can make the code believe there is enough space before calling memcpy. Question is: can we get a call to StreamingBufferAppend()/StreamingBufferAppendNoTrack() with a length value of 3GiB-4GiB?

Actions #4

Updated by Philippe Antoine 8 months ago

  • Status changed from New to In Review
  • Target version changed from TBD to 8.0.0-beta1
Actions #5

Updated by Victor Julien 8 months ago

  • Tracker changed from Security to Bug
  • Status changed from In Review to Resolved
  • Priority changed from High to Normal
  • Severity deleted (MODERATE)
Actions #6

Updated by Victor Julien 8 months ago

  • Private changed from Yes to No
  • Label Needs backport to 7.0 added
Actions #7

Updated by OISF Ticketbot 8 months ago

  • Subtask #6970 added
Actions #8

Updated by OISF Ticketbot 8 months ago

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

Updated by Victor Julien 8 months ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF