fix out-of-bounds read for negative errno in _json_c_strerror

diff --git a/strerror_override.c b/strerror_override.c
index a3dd377a3df11fe55c269368d090385232fbdf5d..dda4a5ba325f1792934ba51beb2fdf4ea56ffe4e 100644 (file)
--- a/strerror_override.c
+++ b/strerror_override.c
@@ -69,6 +69,7 @@ char *_json_c_strerror(int errno_in)
        int start_idx;
        char digbuf[20];
        int ii, jj;
+       unsigned int uerr;
 
        if (!_json_c_strerror_enable)
                _json_c_strerror_enable = (getenv("_JSON_C_STRERROR_ENABLE") == NULL) ? -1 : 1;
@@ -94,14 +95,22 @@ char *_json_c_strerror(int errno_in)
        }
 
        // It's not one of the known errno values, return the numeric value.
-       for (ii = 0; errno_in >= 10; errno_in /= 10, ii++)
+       // Work on the magnitude in unsigned space: a negative errno_in makes
+       // errno_in % 10 negative, which would index before the "0123456789"
+       // literal (an out-of-bounds read).  Computing 0u - errno_in in unsigned
+       // is also well defined for INT_MIN, where -errno_in would overflow.
+       uerr = (errno_in < 0) ? (0u - (unsigned int)errno_in) : (unsigned int)errno_in;
+       for (ii = 0; uerr >= 10; uerr /= 10, ii++)
        {
-               digbuf[ii] = "0123456789"[(errno_in % 10)];
+               digbuf[ii] = "0123456789"[(uerr % 10)];
        }
-       digbuf[ii] = "0123456789"[(errno_in % 10)];
+       digbuf[ii] = "0123456789"[(uerr % 10)];
 
-       // Reverse the digits
-       for (start_idx = sizeof(PREFIX) - 1; ii >= 0; ii--, start_idx++)
+       // Reverse the digits, keeping the sign for negative values
+       start_idx = sizeof(PREFIX) - 1;
+       if (errno_in < 0)
+               errno_buf[start_idx++] = '-';
+       for (; ii >= 0; ii--, start_idx++)
        {
                errno_buf[start_idx] = digbuf[ii];
        }

diff --git a/tests/test_strerror.c b/tests/test_strerror.c
index 773b1aa10b6ceda3d84ed2e9bd2bb1a9dbb22ceb..121f4e0a9521206f31be44e1aab4e3aa32dc67e8 100644 (file)
--- a/tests/test_strerror.c
+++ b/tests/test_strerror.c
@@ -9,5 +9,9 @@ int main(int argc, char **argv)
 {
        puts(strerror(10000));
        puts(strerror(999));
+       /* Negative values must not index before the digit table (out-of-bounds
+        * read); INT_MIN also exercises the unsigned magnitude handling. */
+       puts(strerror(-5));
+       puts(strerror(-2147483647 - 1));
        return 0;
 }

diff --git a/tests/test_strerror.expected b/tests/test_strerror.expected
index b6b3bb655482bd72a3f0bc69291bbf9532f0b504..8b527df09e6af0fbe4e7536e2de059e5973731e9 100644 (file)
--- a/tests/test_strerror.expected
+++ b/tests/test_strerror.expected
@@ -1,2 +1,4 @@
 ERRNO=10000
 ERRNO=999
+ERRNO=-5
+ERRNO=-2147483648