mm2 / Little-CMS

A free, open source, CMM engine. It provides fast transforms between ICC profiles.
https://www.littlecms.com
MIT License
571 stars 176 forks source link

Fixed potentials overflows. #376

Closed kobrineli closed 1 year ago

kobrineli commented 1 year ago

Hi! I've been searching for errors in Little-CMS with sydr-fuzz security predicates and found overflow errors in cmslut.c:574 and cmslut.c:669. I've added some checks to handle them.

mm2 commented 1 year ago

Thanks for reporting. Do you have a sample ICC profile that demonstrates the overflow? Having a sample file is very important, as if the overflow can be triggered when a program loads an ICC profile you have discovered a true bug.

kobrineli commented 1 year ago

@mm2 Hi! I have the inputs, that trigger overflow in cmslut.c:574 and cmslut.c:666

To reproduce errors:

  1. Build and run docker container from here https://github.com/ispras/oss-sydr-fuzz/tree/master/projects/lcms:

    $ sudo docker build -t oss-sydr-fuzz-lcms .
    $ sudo docker run --rm --privileged -v `pwd`:/fuzz -it oss-sydr-fuzz-lcms /bin/bash
  2. Run /lcms_fuzz/cms_overwrite_transform_fuzzer binary on following inputs lcms_overflow_1.txt lcms_overflow_2.txt

    # /lcms_fuzz/cms_overwrite_transform_fuzzer lcms_overflow_1.txt
    # /lcms_fuzz/cms_overwrite_transform_fuzzer lcms_overflow_2.txt
  3. You will see the following outputs:

    cmslut.c:574:42: runtime error: unsigned integer overflow: 15 * 530604000 cannot be represented in type 'unsigned int'
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior cmslut.c:574:42 in
    
    cmslut.c:666:42: runtime error: unsigned integer overflow: 32768 * 279576576 cannot be represented in type 'unsigned int'
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior cmslut.c:666:42 in
mm2 commented 1 year ago

Thanks,

This is harmless, all you can obtain with such profiles is to create a big block of memory that will immediatly be freed as file size does not match. Then the profiles are correctly reported as corrupted.

Cannot be used to do exploits.

I am adding a simple check to avoid the big malloc, but otherwise there is no vulnerability. See 5b083856e928bd73a655c1d6c1255c49d973ceca

Thanks again for reporting the issue

kobrineli commented 1 year ago

So I close the pr?

mm2 commented 1 year ago

So I close the pr?

If you are happy with the solution, yes.

I thank you again for pointing out this issue. The more robust the code, the better and thanks to your report we avoided a huge memory malloc/free movement.

kobrineli commented 1 year ago

@mm2 I suppose the checks from this PR are needed anyway. Your fix prevent the overflow when outputChan is less or equal to 15, but in the cmslut.c:666 as you can see in sanitizer report in the upper comment, the outputChan equals 32768, the CubeSize return result equals 279576576, which is less then (2^32 - 1) / 15 == 286331153, so your check wouldn't work and overflow would still occur.

Moreover, I've checked lcms latest version with your fix. The first overflow in cmslut.c:577 is fixed, but the second one in cmslut.c:669 is still there:

cmslut.c:669:42: runtime error: unsigned integer overflow: 32768 * 279576576 cannot be represented in type 'unsigned int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior cmslut.c:669:42 in
mm2 commented 1 year ago

This is a completly different thing with a different origin. I have added a check as well. c61a2c798e0f3ca73d2f78e8721ae37e6a70c045

kobrineli commented 1 year ago

@mm2 I've checked your fix on overflow inputs, now that works. I close the PR