Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiply integer overflow in AllocateDataSet #171

Closed
quangnh89 opened this issue Aug 14, 2018 · 3 comments
Closed

Multiply integer overflow in AllocateDataSet #171

quangnh89 opened this issue Aug 14, 2018 · 3 comments

Comments

@quangnh89
Copy link

quangnh89 commented Aug 14, 2018

VULNERABILITY DETAILS

I have audited source code of lcms library and I have founded a vulnerability in AllocateDataSet function (cmscgats.c). The attached it8 could crash lcms when ASAN was enabled on Linux. This vulnerability is assigned CVE-2018-16435.

void AllocateDataSet(cmsIT8* it8)
{
    TABLE* t = GetTable(it8);

    if (t -> Data) return;    // Already allocated

    t-> nSamples   = atoi(cmsIT8GetProperty(it8, "NUMBER_OF_FIELDS"));
    t-> nPatches   = atoi(cmsIT8GetProperty(it8, "NUMBER_OF_SETS"));

    t-> Data = (char**)AllocChunk (it8, ((cmsUInt32Number) t->nSamples + 1) * ((cmsUInt32Number) t->nPatches + 1) *sizeof (char*)); // <<------ Vuln here 
    if (t->Data == NULL) {

        SynError(it8, "AllocateDataSet: Unable to allocate data array");
    }
}

if nSamples is 2 and nPatches is 0x55555555, ((cmsUInt32Number) t->nSamples + 1) * ((cmsUInt32Number) t->nPatches + 1) *sizeof (char*) is larger than the maximum representable value (0xffffffff). The result of an overflow is that the least significant representable bits of the result are stored. Data will point to a small memory region and can not use to store large data.

REPRODUCTION CASE

Following code will trigger crash

#include <stdio.h>
#include <lcms2.h>
#include "lcms2_internal.h"

int main(int argc, char* argv[]){
    cmsIT8LoadFromFile(NULL, "AllocateDataSet.crash.IT8");
    return 0;
}

ASAN Log:

=================================================================
==12907==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62a000005200 at pc 0x000000592bc5 bp 0x7ffd5f5c46c0 sp 0x7ffd5f5c46b8
WRITE of size 8 at 0x62a000005200 thread T0
    #0 0x592bc4 in SetData /root/lcms2/lcms2-asan/src/cmscgats.c:1549:45
    #1 0x5986e3 in DataSection /root/lcms2/lcms2-asan/src/cmscgats.c:1894:18
    #2 0x590259 in ParseIT8 /root/lcms2/lcms2-asan/src/cmscgats.c:2070:26
    #3 0x5915e6 in cmsIT8LoadFromFile /root/lcms2/lcms2-asan/src/cmscgats.c:2365:10
    #4 0x4eb86b in main /root/lcms2/test.c:7:5
    #5 0x7f9a0b6b682f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #6 0x41abb8 in _start (/root/lcms2/test+0x41ab68)

0x62a000005200 is located 0 bytes to the right of 20480-byte region [0x62a000000200,0x62a000005200)
allocated by thread T0 here:
    #0 0x4bb2e3 in __interceptor_malloc /tmp/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:88:3
    #1 0x4ebca8 in _cmsMallocZero /root/lcms2/lcms2-asan/src/cmserr.c:88:15
    #2 0x59474c in AllocBigBlock /root/lcms2/lcms2-asan/src/cmscgats.c:1053:17
    #3 0x58b62c in AllocChunk /root/lcms2/lcms2-asan/src/cmscgats.c:1095:52
    #4 0x58af66 in cmsIT8Alloc /root/lcms2/lcms2-asan/src/cmscgats.c:1310:35
    #5 0x5913e2 in cmsIT8LoadFromFile /root/lcms2/lcms2-asan/src/cmscgats.c:2349:13
    #6 0x4eb86b in main /root/lcms2/test.c:7:5
    #7 0x7f9a0b6b682f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: heap-buffer-overflow /root/lcms2/lcms2-asan/src/cmscgats.c:1549:45 in SetData
Shadow bytes around the buggy address:
  0x0c547fff89f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c547fff8a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c547fff8a10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c547fff8a20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c547fff8a30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c547fff8a40:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c547fff8a50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c547fff8a60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c547fff8a70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c547fff8a80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c547fff8a90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==12907==ABORTING

PATCH

--- a/src/cmscgats.c
+++ b/src/cmscgats.c
@@ -1504,6 +1504,16 @@
     t-> nSamples   = atoi(cmsIT8GetProperty(it8, "NUMBER_OF_FIELDS"));
     t-> nPatches   = atoi(cmsIT8GetProperty(it8, "NUMBER_OF_SETS"));
 
+#if (UINT_MAX == 4294967295U)
+    const cmsUInt32Number UInt32Max = UINT_MAX;
+#elif (ULONG_MAX == 4294967295U)
+    const cmsUInt32Number UInt32Max = ULONG_MAX;
+#endif
+    if (((cmsUInt32Number) t->nSamples + 1) >  (UInt32Max / sizeof (char*)) / ((cmsUInt32Number) t->nPatches + 1)) {
+        SynError(it8, "AllocateDataSet: Unable to allocate data array");
+        return;
+    }
+
     t-> Data = (char**)AllocChunk (it8, ((cmsUInt32Number) t->nSamples + 1) * ((cmsUInt32Number) t->nPatches + 1) *sizeof (char*));
     if (t->Data == NULL) {

I have checked the lastest version of LCMS. The vulnerability still exists.

https://github.com/mm2/Little-CMS/blob/master/src/cmscgats.c#L1509

Testcase:
AllocateDataSet.crash.IT8.txt

@quangnh89 quangnh89 changed the title Heap Buffer Overflow in AllocateDataSet Multiply integer overflow in AllocateDataSet Aug 14, 2018
@quangnh89
Copy link
Author

hi @mm2 . I have reviewed this commit (768f70c) and I understand that this issue is resolved. May you confirm it?

@mm2
Copy link
Owner

mm2 commented Aug 16, 2018

Hi, thanks for your detailed report and for the patch. Having examined the source, it seems to me it is safer and makes more sense to limit the amount of data that a CGATS file can hold. I have modified the code to deal with such situation.
Thanks again!

@mm2 mm2 closed this as completed Aug 16, 2018
@quangnh89
Copy link
Author

This vulnerability is assigned CVE-2018-16435.

buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this issue Sep 5, 2018
Little CMS (aka Little Color Management System) 2.9 has an integer overflow
in the AllocateDataSet function in cmscgats.c, leading to a heap-based
buffer overflow in the SetData function via a crafted file in the second
argument to cmsIT8LoadFromFile.

For more details, see:
mm2/Little-CMS#171
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-16435

The upstream fix unfortunately includes a number of unrelated changes, but
thse files are not used when building for Linux.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this issue Sep 28, 2018
Little CMS (aka Little Color Management System) 2.9 has an integer overflow
in the AllocateDataSet function in cmscgats.c, leading to a heap-based
buffer overflow in the SetData function via a crafted file in the second
argument to cmsIT8LoadFromFile.

For more details, see:
mm2/Little-CMS#171
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-16435

The upstream fix unfortunately includes a number of unrelated changes, but
thse files are not used when building for Linux.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
(cherry picked from commit 9f81f57)
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this issue Sep 28, 2018
Little CMS (aka Little Color Management System) 2.9 has an integer overflow
in the AllocateDataSet function in cmscgats.c, leading to a heap-based
buffer overflow in the SetData function via a crafted file in the second
argument to cmsIT8LoadFromFile.

For more details, see:
mm2/Little-CMS#171
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-16435

The upstream fix unfortunately includes a number of unrelated changes, but
thse files are not used when building for Linux.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
(cherry picked from commit 9f81f57)
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants