Actions
Bug #6903
closedstreaming buffer: heap overflows in StreamingBufferAppend()/StreamingBufferAppendNoTrack()
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
Updated by Philippe Antoine 7 months ago
Is there some limit of 1Gbyte for streaming buffers ? This reminds me of something...
Updated by Victor Julien 7 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?
Updated by Philippe Antoine 7 months ago
- Status changed from New to In Review
- Target version changed from TBD to 8.0.0-beta1
Updated by Victor Julien 7 months ago
- Tracker changed from Security to Bug
- Status changed from In Review to Resolved
- Priority changed from High to Normal
- Severity deleted (
MODERATE)
Updated by Victor Julien 7 months ago
- Private changed from Yes to No
- Label Needs backport to 7.0 added
Actions