nothings / stb

stb single-file public domain libraries for C/C++
https://twitter.com/nothings
Other
25.8k stars 7.66k forks source link

stb_image_write.h: UBSAN, left shift of negative value #1433

Open BlackMATov opened 1 year ago

BlackMATov commented 1 year ago

Hello! UBSan complains about a left shift of negative value:

stb_image_write.h:1263:14: runtime error: left shift of negative value -402661120
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior stb_image_write.h:1263:14
ChrisThrasher commented 8 months ago
.../sfml/extlibs/headers/stb_image/stb_image_write.h:1263:14: runtime error: left shift of negative value -521666560
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior .../sfml/extlibs/headers/stb_image/stb_image_write.h:1263:14 

I've encountered this as well. I'm using std_image_write version 1.16. It appears to happen when calling stbi_write_jpg.

static void stbiw__jpg_writeBits(stbi__write_context *s, int *bitBufP, int *bitCntP, const unsigned short *bs) {
   int bitBuf = *bitBufP, bitCnt = *bitCntP;
   bitCnt += bs[1];
   bitBuf |= bs[0] << (24 - bitCnt);
   while(bitCnt >= 8) {
      unsigned char c = (bitBuf >> 16) & 255;
      stbiw__putc(s, c);
      if(c == 255) {
         stbiw__putc(s, 0);
      }
      bitBuf <<= 8; // <-- This line is where UBSan traps
      bitCnt -= 8;
   }
   *bitBufP = bitBuf;
   *bitCntP = bitCnt;
}
ChrisThrasher commented 8 months ago

When trying to call stbi_load_from_callbacks, clang-tidy-17 warns about potential undefined behavior. We use this function when trying to encode JPEGs so this may be related to the UB we're seeing in stbiw__jpg_writeBits.

/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1615:7: error: Undefined or garbage value returned to caller [clang-analyzer-core.uninitialized.UndefReturn,-warnings-as-errors]
 1615 |       return *s->img_buffer++;
      |       ^
/Users/thrasher/Projects/sfml/src/SFML/Graphics/ImageLoader.cpp:159:9: note: Assuming the condition is false
  159 |     if (stream.seek(0) == -1)
      |         ^~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/src/SFML/Graphics/ImageLoader.cpp:159:5: note: Taking false branch
  159 |     if (stream.seek(0) == -1)
      |     ^
/Users/thrasher/Projects/sfml/src/SFML/Graphics/ImageLoader.cpp:192:31: note: Calling 'stbi_load_from_callbacks'
  192 |     unsigned char* ptr      = stbi_load_from_callbacks(&callbacks, &stream, &width, &height, &channels, STBI_rgb_alpha);
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1437:4: note: Calling 'stbi__start_callbacks'
 1437 |    stbi__start_callbacks(&s, (stbi_io_callbacks *) clbk, user);
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:842:4: note: Calling 'stbi__refill_buffer'
  842 |    stbi__refill_buffer(s);
      |    ^~~~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1597:12: note: Calling '__invoke'
 1597 |    int n = (s->io.read)(s->io_user_data,(char*)s->buffer_start,s->buflen);
      |            ^~~~~~~~~~~~~~~~~~~~~~30209 warnings generated.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1597:12: note: Returning from '__invoke'
 1597 |    int n = (s->io.read)(s->io_user_data,(char*)s->buffer_start,s->buflen);
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1599:8: note: Assuming 'n' is not equal to 0
 1599 |    if (n == 0) {
      |        ^~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1599:4: note: Taking false branch
 1599 |    if (n == 0) {
      |    ^
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:842:4: note: Returning from 'stbi__refill_buffer'
  842 |    stbi__refill_buffer(s);
      |    ^~~~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1437:4: note: Returning from 'stbi__start_callbacks'
 1437 |    stbi__start_callbacks(&s, (stbi_io_callbacks *) clbk, user);
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1438:11: note: Calling 'stbi__load_and_postprocess_8bit'
 1438 |    return stbi__load_and_postprocess_8bit(&s,x,y,comp,req_comp);
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1261:19: note: Calling 'stbi__load_main'
 1261 |    void *result = stbi__load_main(s, x, y, comp, req_comp, &ri, 8);
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1145:8: note: Calling 'stbi__png_test'
 1145 |    if (stbi__png_test(s))  return stbi__png_load(s,x,y,comp,req_comp, ri);
      |        ^~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:5304:8: note: Calling 'stbi__check_png_header'
 5304 |    r = stbi__check_png_header(s);
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:4603:4: note: Loop condition is true.  Entering loop body
 4603 |    for (i=0; i < 8; ++i)
      |    ^
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:4604:11: note: Calling 'stbi__get8'
 4604 |       if (stbi__get8(s) != png_sig[i]) return stbi__err("bad png sig","Not a PNG");
      |           ^~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1614:8: note: Assuming field 'img_buffer' is < field 'img_buffer_end'
 1614 |    if (s->img_buffer < s->img_buffer_end)
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1614:4: note: Taking true branch
 1614 |    if (s->img_buffer < s->img_buffer_end)
      |    ^
/Users/thrasher/Projects/sfml/extlibs/headers/stb_image/stb_image.h:1615:7: note: Undefined or garbage value returned to caller
 1615 |       return *s->img_buffer++;
      |       ^      ~~~~~~~~~~~~~~~~
uecker commented 1 week ago

Should probably be: bitBuf &= 0x0000FFFF; bitBuf <<= 8;