From: DRC <information@libjpeg-turbo.org>
Date: Wed, 10 Apr 2019 19:28:47 +0000 (-0500)
Subject: jdhuff.c: Silence UBSan signed int overflow error
X-Git-Tag: 2.0.3~24
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d3a3a73f64041c6a6905faf6f9f9832e735fd880;p=libjpeg-turbo

jdhuff.c: Silence UBSan signed int overflow error

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
---

diff --git a/jdhuff.c b/jdhuff.c
index 95f38e5..334ba63 100644
--- 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) */