Security: size_t overflow in jas_stream_peek

[xmoezzz]

Hi, I am writing to report a size_t overflow that I discovered in this project.

1. Vulnerability Summary:

/root/build/jasper-4.2.8/src/libjasper/base/jas_stream.c:713:22: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'size_t' (aka 'unsigned long')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /root/build/jasper-4.2.8/src/libjasper/base/jas_stream.c:713:22 in

2. Affected Version
   The latest release version 4.2.8.

3. Reproduce

• Command line:

jasper "--input" "A" "--output" "B" "--input-format" "pg" "--output-format" "jpc" "" ""

• Compile the executable binary with "--fsanitizer=undefined" flag.
• Corrupted input data A (unzip A.zip first):

A.zip

Best regards,

[mdadams]

@xmoezzz: I cannot reproduce this problem with the latest commit of JasPer on the master branch. When I run the jasper program (built with ASan and UBSan enabled) with the command line that you specified, I get the following output:

warning: ignoring bogus command line argument 
warning: ignoring bogus command line argument 
invalid PGX signature
cannot get header
jas_image_decode: decode operation failed
error: cannot load image data

Did you try the latest commit on the master branch?

[xmoezzz]

Thanks for your reply!
I tried the latest commit of the master branch (7576bf0).

This is my build command:

CC=clang CXX=clang++ \
cmake -DCMAKE_BUILD_TYPE=Debug \
      -DCMAKE_C_FLAGS="-fsanitize=address -fno-omit-frame-pointer -fsanitize=signed-integer-overflow -fsanitize=unsigned-integer-overflow -fsanitize=shift -fsanitize=bounds -fsanitize=null -fsanitize=pointer-overflow -g -O1" \
      -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined" \
      -DCMAKE_SHARED_LINKER_FLAGS="-fsanitize=address,undefined" \
      ../jasper

[mdadams]

@xmoezzz Unsigned integer overflow is not UB. It is well defined with wraparound semantics. Also, this code behaves correctly when overflow/wraparound occurs. So, there is really no bug here. In any case, I have rewritten the loop to avoid any possible wraparound. This should get rid of your sanitizer warning, which is a false positive in this case.

[jubalh]

I think this issue/change was not mentioned in the NEWS document and thus not in 4.2.9 release.

[mdadams]

@jubalh Please note that this change did not fix any real problem. There was not any security vulnerability (or even bug). Unsigned integer overflow is not UB in C and the code worked correctly in the presence of overflow. I only made this change for the benefit of people who might turn on UBSan and get annoyed by benign warnings.

[jubalh]

I see. Thank you for clarifying :)