]> granicus.if.org Git - libjpeg-turbo/commitdiff
jdhuff.c: Silence UBSan signed int overflow error
authorDRC <information@libjpeg-turbo.org>
Wed, 10 Apr 2019 19:28:47 +0000 (14:28 -0500)
committerDRC <information@libjpeg-turbo.org>
Wed, 10 Apr 2019 19:57:12 +0000 (14:57 -0500)
Some pathological test images have been created that can cause s to
overflow or underflow the signed int data type during decompression.
This is technically undefined behavior according to the C spec, although
every modern implementation I'm aware of will treat the signed int as a
2's complement unsigned int, thus causing the value to wrap around to
INT_MIN if it exceeds INT_MAX.  This commit simply makes that behavior
explicit in order to shut up UBSan.  At least when building for x86-64
or i386 using Clang or GCC, this commit does not change the
compiler-generated assembly code at all.

The code that triggered this error has existed in the libjpeg code base
for at least 20 years (and probably much longer), so the fact that it
hasn't produced a user-visible problem in all of that time strongly
suggests that UBSan is being overly pedantic here.  But if someone can
cough up a platform that doesn't wrap around to INT_MIN when 1 is added
to INT_MAX, then I'll happily change my opinion.

Fixes #347

jdhuff.c

index 95f38e547e401d3e353100c1db657b924c2f511a..334ba63b5cb88e10012a5e601278c12b278bb24d 100644 (file)
--- a/jdhuff.c
+++ b/jdhuff.c
@@ -4,7 +4,7 @@
  * This file was part of the Independent JPEG Group's software:
  * Copyright (C) 1991-1997, Thomas G. Lane.
  * libjpeg-turbo Modifications:
- * Copyright (C) 2009-2011, 2016, 2018, D. R. Commander.
+ * Copyright (C) 2009-2011, 2016, 2018-2019, D. R. Commander.
  * For conditions of distribution and use, see the accompanying README.ijg
  * file.
  *
@@ -589,7 +589,11 @@ decode_mcu_slow(j_decompress_ptr cinfo, JBLOCKROW *MCU_data)
     if (entropy->dc_needed[blkn]) {
       /* Convert DC difference to actual value, update last_dc_val */
       int ci = cinfo->MCU_membership[blkn];
-      s += state.last_dc_val[ci];
+      /* This is really just
+       *   s += state.last_dc_val[ci];
+       * It is written this way in order to shut up UBSan.
+       */
+      s = (int)((unsigned int)s + (unsigned int)state.last_dc_val[ci]);
       state.last_dc_val[ci] = s;
       if (block) {
         /* Output the DC coefficient (assumes jpeg_natural_order[0] = 0) */