From 9a09618471b08e3c18750783637a2a7f56757aab Mon Sep 17 00:00:00 2001
From: Mamoru TASAKA <mtasaka@fedoraproject.org>
Date: Wed, 22 Dec 2021 08:32:25 +0900
Subject: [PATCH] decode_long_array: avoid overflow on 32 bit when overread

In grib_decode_unsigned_long and so on, when input argument nbits is 32
(which should be supported on 32bit) and in the case
"bitsToRead might now be negative (too many bits read)" happened,
once the value "ret" can overflow 32 bit range, later with
the operation "ret >>= -1 * bitsToRead;", "ret" gets fit in 32 bit value.

So during the calculation of "ret" value, preserve the "ret" value
which is to be carried up on left shift, and the last put back the
carried up value to ret again on right shift.

Note that even on 64 bit, currently when input argument nbit is 64 and
when "too many bits read" happened, grib_decode_unsigned_long and so on
don't behave correctly for now. This change will also fix this issue.
---
 src/grib_bits_any_endian.c        |  8 ++++++++
 src/grib_bits_any_endian_omp.c    | 13 +++++++++++--
 src/grib_bits_any_endian_simple.c | 13 +++++++++++--
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/src/grib_bits_any_endian.c b/src/grib_bits_any_endian.c
index 411d902b..7fdb682c 100644
--- a/src/grib_bits_any_endian.c
+++ b/src/grib_bits_any_endian.c
@@ -178,6 +178,7 @@ char* grib_decode_string(const unsigned char* bitStream, long* bitOffset, size_t
 unsigned long grib_decode_unsigned_long(const unsigned char* p, long* bitp, long nbits)
 {
     unsigned long ret    = 0;
+    unsigned long ret_carry_up = 0;
     long oc              = *bitp / 8;
     unsigned long mask   = 0;
     long pi              = 0;
@@ -222,6 +223,8 @@ unsigned long grib_decode_unsigned_long(const unsigned char* p, long* bitp, long
     /* read at least enough bits (byte by byte) from input */
     bitsToRead = nbits;
     while (bitsToRead > 0) {
+        ret_carry_up <<= 8;
+        ret_carry_up |= (ret >> (max_nbits - 8));
         ret <<= 8;
         /*   ret += p[pi];     */
         DebugAssert((ret & p[pi]) == 0);
@@ -235,6 +238,7 @@ unsigned long grib_decode_unsigned_long(const unsigned char* p, long* bitp, long
     /* bitsToRead might now be negative (too many bits read) */
     /* remove those which are too much */
     ret >>= -1 * bitsToRead;
+    if (bitsToRead < 0) ret |= (ret_carry_up << (max_nbits - (-1 * bitsToRead)));
     /* remove leading bits (from previous value) */
     ret &= mask;
     /* printf("%d %d\n", ret2, ret);*/
@@ -310,6 +314,7 @@ int grib_encode_unsigned_long(unsigned char* p, unsigned long val, long* bitp, l
 size_t grib_decode_size_t(const unsigned char* p, long* bitp, long nbits)
 {
     size_t ret           = 0;
+    size_t ret_carry_up  = 0;
     long oc              = *bitp / 8;
     size_t mask          = 0;
     long pi              = 0;
@@ -346,6 +351,8 @@ size_t grib_decode_size_t(const unsigned char* p, long* bitp, long nbits)
     /* read at least enough bits (byte by byte) from input */
     bitsToRead = nbits;
     while (bitsToRead > 0) {
+        ret_carry_up <<= 8;
+        ret_carry_up |= (ret >> (max_nbits_size_t - 8));
         ret <<= 8;
         /*   ret += p[pi];     */
         DebugAssert((ret & p[pi]) == 0);
@@ -359,6 +366,7 @@ size_t grib_decode_size_t(const unsigned char* p, long* bitp, long nbits)
     /* bitsToRead might now be negative (too many bits read) */
     /* remove those which are too much */
     ret >>= -1 * bitsToRead;
+    if (bitsToRead < 0) ret |= (ret_carry_up << (max_nbits_size_t - (-1 * bitsToRead)));
     /* remove leading bits (from previous value) */
     ret &= mask;
 
diff --git a/src/grib_bits_any_endian_omp.c b/src/grib_bits_any_endian_omp.c
index fa726a19..cca07e22 100644
--- a/src/grib_bits_any_endian_omp.c
+++ b/src/grib_bits_any_endian_omp.c
@@ -34,8 +34,11 @@ int grib_decode_long_array(const unsigned char* p, long* bitp, long bitsPerValue
     for (i = 0; i < n_vals; i++) {
         /* read at least enough bits (byte by byte) from input */
         long bitsToRead = bitsPerValue;
-        long ret        = 0;
+        unsigned long ret        = 0;
+        unsigned long ret_carry_up = 0;
         while (bitsToRead > 0) {
+            ret_carry_up <<= 8;
+            ret_carry_up |= (ret >> (max_nbits - 8));
             ret <<= 8;
             /*   ret += p[pi];         */
             /*   Assert( (ret & p[pi]) == 0 ); */
@@ -49,9 +52,10 @@ int grib_decode_long_array(const unsigned char* p, long* bitp, long bitsPerValue
         /* bitsToRead might now be negative (too many bits read) */
         /* remove those which are too much */
         ret >>= -1 * bitsToRead;
+        if (bitsToRead < 0) ret |= (ret_carry_up << (max_nbits - (-1 * bitsToRead)));
         /* remove leading bits (from previous value) */
         ret &= mask;
-        val[i] = ret;
+        val[i] = (long)ret;
 
         usefulBitsInByte = -1 * bitsToRead; /* prepare for next round */
         if (usefulBitsInByte > 0) {
@@ -79,6 +83,7 @@ int grib_decode_double_array(const unsigned char* p, long* bitp, long bitsPerVal
 {
     long i               = 0;
     unsigned long lvalue = 0;
+    unsigned long lvalue_carry_up = 0;
     double x;
 
 #if 0
@@ -107,9 +112,12 @@ int grib_decode_double_array(const unsigned char* p, long* bitp, long bitsPerVal
         /* value read as long */
         long bitsToRead = 0;
         lvalue          = 0;
+        lvalue_carry_up = 0;
         bitsToRead      = bitsPerValue;
         /* read one byte after the other to lvalue until >= bitsPerValue are read */
         while (bitsToRead > 0) {
+            lvalue_carry_up <<= 8;
+            lvalue_carry_up |= (lvalue >> (max_nbits - 8));
             lvalue <<= 8;
             lvalue += p[pi];
             pi++;
@@ -119,6 +127,7 @@ int grib_decode_double_array(const unsigned char* p, long* bitp, long bitsPerVal
         *bitp += bitsPerValue;
         /* bitsToRead is now <= 0, remove the last bits */
         lvalue >>= -1 * bitsToRead;
+        if (bitsToRead < 0) lvalue |= (lvalue_carry_up << (max_nbits - (-1 * bitsToRead)));
         /* set leading bits to 0 - removing bits used for previous number */
         lvalue &= mask;
 
diff --git a/src/grib_bits_any_endian_simple.c b/src/grib_bits_any_endian_simple.c
index 7c00aa4c..6c08594f 100644
--- a/src/grib_bits_any_endian_simple.c
+++ b/src/grib_bits_any_endian_simple.c
@@ -40,8 +40,11 @@ int grib_decode_long_array(const unsigned char* p, long* bitp, long bitsPerValue
     for (i = 0; i < n_vals; i++) {
         /* read at least enough bits (byte by byte) from input */
         long bitsToRead = bitsPerValue;
-        long ret        = 0;
+        unsigned long ret        = 0;
+        unsigned long ret_carry_up = 0;
         while (bitsToRead > 0) {
+            ret_carry_up <<= 8;
+            ret_carry_up |= (ret >> (max_nbits - 8));
             ret <<= 8;
             /*   ret += p[pi];         */
             /*   Assert( (ret & p[pi]) == 0 ); */
@@ -55,9 +58,10 @@ int grib_decode_long_array(const unsigned char* p, long* bitp, long bitsPerValue
         /* bitsToRead might now be negative (too many bits read) */
         /* remove those which are too much */
         ret >>= -1 * bitsToRead;
+        if (bitsToRead < 0) ret |= (ret_carry_up << (max_nbits - (-1 * bitsToRead)));
         /* remove leading bits (from previous value) */
         ret &= mask;
-        val[i] = ret;
+        val[i] = (long)ret;
 
         usefulBitsInByte = -1 * bitsToRead; /* prepare for next round */
         if (usefulBitsInByte > 0) {
@@ -85,6 +89,7 @@ int grib_decode_double_array(const unsigned char* p, long* bitp, long bitsPerVal
 {
     long i               = 0;
     unsigned long lvalue = 0;
+    unsigned long lvalue_carry_up = 0;
     double x;
 
 #if 0
@@ -134,9 +139,12 @@ int grib_decode_double_array(const unsigned char* p, long* bitp, long bitsPerVal
             /* value read as long */
             long bitsToRead = 0;
             lvalue          = 0;
+            lvalue_carry_up = 0;
             bitsToRead      = bitsPerValue;
             /* read one byte after the other to lvalue until >= bitsPerValue are read */
             while (bitsToRead > 0) {
+                lvalue_carry_up <<= 8;
+                lvalue_carry_up |= (lvalue >> (max_nbits - 8));
                 lvalue <<= 8;
                 lvalue += p[pi];
                 pi++;
@@ -146,6 +154,7 @@ int grib_decode_double_array(const unsigned char* p, long* bitp, long bitsPerVal
             *bitp += bitsPerValue;
             /* bitsToRead is now <= 0, remove the last bits */
             lvalue >>= -1 * bitsToRead;
+            if (bitsToRead < 0) lvalue |= (lvalue_carry_up << (max_nbits - (-1 * bitsToRead)));
             /* set leading bits to 0 - removing bits used for previous number */
             lvalue &= mask;
 
-- 
2.33.1

