Fix XER decoder of INTEGER, Issue #344.

    In some cases an INTEGER overflow during parsing is not detected
    and incorrect value is returned to the decoder instead of an error.
    Reported by Nika Pona <npona@digamma.ai>.
    (Severity: low; Seuciry impact: medium).

ChangeLog

@@ -35,6 +35,11 @@
         (Severity: low; Security impact: none)
       * Fix XER decoder crash on maliciously constructed ENUMERATED input.
         (Severity: medium; Security impact: medium)
+      * Fix XER decoder of INTEGER, OBJECT IDENTIFIER, and RELATIVE-OID.
+        In some cases an INTEGER overflow during parsing is not detected
+        and incorrect value is returned to the decoder instead of an error.
+        Reported by Nika Pona <npona@digamma.ai>.
+        (Severity: low; Security impact: medium).
 
     FIXES IN TOOLING:
       * CVE-2017-12966 verified not present.

skeletons/INTEGER.c

-/*-
- * Copyright (c) 2003-2014 Lev Walkin <vlm@lionet.info>.
+/*
+ * Copyright (c) 2003-2019 Lev Walkin <vlm@lionet.info>.
  * All rights reserved.
  * Redistribution and modifications are permitted subject to BSD license.
  */
@@ -1032,64 +1032,71 @@ asn_ulong2INTEGER(INTEGER_t *st, unsigned long value) {
  */
 enum asn_strtox_result_e
 asn_strtoimax_lim(const char *str, const char **end, intmax_t *intp) {
-	int sign = 1;
-	intmax_t value;
+    int sign = 1;
+    intmax_t value;
 
-#define ASN1_INTMAX_MAX ((~(uintmax_t)0) >> 1)
-    const intmax_t upper_boundary = ASN1_INTMAX_MAX / 10;
-	intmax_t last_digit_max = ASN1_INTMAX_MAX % 10;
-#undef  ASN1_INTMAX_MAX
+    const intmax_t asn1_intmax_max = ((~(uintmax_t)0) >> 1);
+    const intmax_t upper_boundary = asn1_intmax_max / 10;
+    intmax_t last_digit_max = asn1_intmax_max % 10;
 
-	if(str >= *end) return ASN_STRTOX_ERROR_INVAL;
+    if(str >= *end) return ASN_STRTOX_ERROR_INVAL;
 
-	switch(*str) {
-	case '-':
-		last_digit_max++;
-		sign = -1;
-		/* FALL THROUGH */
-	case '+':
-		str++;
-		if(str >= *end) {
-			*end = str;
-			return ASN_STRTOX_EXPECT_MORE;
-		}
-	}
+    switch(*str) {
+    case '-':
+        last_digit_max++;
+        sign = -1;
+        /* FALL THROUGH */
+    case '+':
+        str++;
+        if(str >= *end) {
+            *end = str;
+            return ASN_STRTOX_EXPECT_MORE;
+        }
+    }
 
-	for(value = 0; str < (*end); str++) {
-		switch(*str) {
-		case 0x30: case 0x31: case 0x32: case 0x33: case 0x34:
-		case 0x35: case 0x36: case 0x37: case 0x38: case 0x39: {
-			int d = *str - '0';
-			if(value < upper_boundary) {
-				value = value * 10 + d;
-			} else if(value == upper_boundary) {
-				if(d <= last_digit_max) {
-					if(sign > 0) {
-						value = value * 10 + d;
-					} else {
-						sign = 1;
-						value = -value * 10 - d;
-					}
-				} else {
-					*end = str;
-					return ASN_STRTOX_ERROR_RANGE;
-				}
-			} else {
-				*end = str;
-				return ASN_STRTOX_ERROR_RANGE;
-			}
-		    }
-		    continue;
-		default:
-		    *end = str;
-		    *intp = sign * value;
-		    return ASN_STRTOX_EXTRA_DATA;
-		}
-	}
+    for(value = 0; str < (*end); str++) {
+        if(*str >= 0x30 && *str <= 0x39) {
+            int d = *str - '0';
+            if(value < upper_boundary) {
+                value = value * 10 + d;
+            } else if(value == upper_boundary) {
+                if(d <= last_digit_max) {
+                    if(sign > 0) {
+                        value = value * 10 + d;
+                    } else {
+                        sign = 1;
+                        value = -value * 10 - d;
+                    }
+                    str += 1;
+                    if(str < *end) {
+                        // If digits continue, we're guaranteed out of range.
+                        *end = str;
+                        if(*str >= 0x30 && *str <= 0x39) {
+                            return ASN_STRTOX_ERROR_RANGE;
+                        } else {
+                            *intp = sign * value;
+                            return ASN_STRTOX_EXTRA_DATA;
+                        }
+                    }
+                    break;
+                } else {
+                    *end = str;
+                    return ASN_STRTOX_ERROR_RANGE;
+                }
+            } else {
+                *end = str;
+                return ASN_STRTOX_ERROR_RANGE;
+            }
+        } else {
+            *end = str;
+            *intp = sign * value;
+            return ASN_STRTOX_EXTRA_DATA;
+        }
+    }
 
-	*end = str;
-	*intp = sign * value;
-	return ASN_STRTOX_OK;
+    *end = str;
+    *intp = sign * value;
+    return ASN_STRTOX_OK;
 }
 
 /*
@@ -1100,56 +1107,63 @@ asn_strtoimax_lim(const char *str, const char **end, intmax_t *intp) {
  */
 enum asn_strtox_result_e
 asn_strtoumax_lim(const char *str, const char **end, uintmax_t *uintp) {
-	uintmax_t value;
+    uintmax_t value;
 
-#define ASN1_UINTMAX_MAX ((~(uintmax_t)0))
-    const uintmax_t upper_boundary = ASN1_UINTMAX_MAX / 10;
-    uintmax_t last_digit_max = ASN1_UINTMAX_MAX % 10;
-#undef ASN1_UINTMAX_MAX
+    const uintmax_t asn1_uintmax_max = ((~(uintmax_t)0));
+    const uintmax_t upper_boundary = asn1_uintmax_max / 10;
+    uintmax_t last_digit_max = asn1_uintmax_max % 10;
 
     if(str >= *end) return ASN_STRTOX_ERROR_INVAL;
 
-	switch(*str) {
-	case '-':
+    switch(*str) {
+    case '-':
         return ASN_STRTOX_ERROR_INVAL;
-	case '+':
-		str++;
-		if(str >= *end) {
-			*end = str;
-			return ASN_STRTOX_EXPECT_MORE;
-		}
-	}
+    case '+':
+        str++;
+        if(str >= *end) {
+            *end = str;
+            return ASN_STRTOX_EXPECT_MORE;
+        }
+    }
 
-	for(value = 0; str < (*end); str++) {
-		switch(*str) {
-		case 0x30: case 0x31: case 0x32: case 0x33: case 0x34:
-		case 0x35: case 0x36: case 0x37: case 0x38: case 0x39: {
-			unsigned int d = *str - '0';
-			if(value < upper_boundary) {
-				value = value * 10 + d;
-			} else if(value == upper_boundary) {
-				if(d <= last_digit_max) {
+    for(value = 0; str < (*end); str++) {
+        if(*str >= 0x30 && *str <= 0x39) {
+            unsigned int d = *str - '0';
+            if(value < upper_boundary) {
+                value = value * 10 + d;
+            } else if(value == upper_boundary) {
+                if(d <= last_digit_max) {
                     value = value * 10 + d;
+                    str += 1;
+                    if(str < *end) {
+                        // If digits continue, we're guaranteed out of range.
+                        *end = str;
+                        if(*str >= 0x30 && *str <= 0x39) {
+                            return ASN_STRTOX_ERROR_RANGE;
+                        } else {
+                            *uintp = value;
+                            return ASN_STRTOX_EXTRA_DATA;
+                        }
+                    }
+                    break;
                 } else {
-					*end = str;
-					return ASN_STRTOX_ERROR_RANGE;
-				}
-			} else {
-				*end = str;
-				return ASN_STRTOX_ERROR_RANGE;
-			}
-		    }
-		    continue;
-		default:
-		    *end = str;
-		    *uintp = value;
-		    return ASN_STRTOX_EXTRA_DATA;
-		}
-	}
+                    *end = str;
+                    return ASN_STRTOX_ERROR_RANGE;
+                }
+            } else {
+                *end = str;
+                return ASN_STRTOX_ERROR_RANGE;
+            }
+        } else {
+            *end = str;
+            *uintp = value;
+            return ASN_STRTOX_EXTRA_DATA;
+        }
+    }
 
-	*end = str;
-	*uintp = value;
-	return ASN_STRTOX_OK;
+    *end = str;
+    *uintp = value;
+    return ASN_STRTOX_OK;
 }
 
 enum asn_strtox_result_e

tests/tests-skeletons/check-INTEGER.c

@@ -178,6 +178,129 @@ check_xer(int lineno, int tofail, char *xmldata, long orig_value) {
 	ASN_STRUCT_FREE(asn_DEF_INTEGER, st);
 }
 
+static void
+check_strtoimax() {
+    const intmax_t intmax_max = ((~(uintmax_t)0) >> 1);
+    const intmax_t intmax_min = -((intmax_t)intmax_max) - 1;
+    char positive_max[32];
+    char negative_min[32];
+    const int len_pmax = snprintf(positive_max, sizeof(positive_max),
+        "+%" ASN_PRIdMAX, intmax_max);
+    const int len_nmin = snprintf(negative_min, sizeof(negative_min),
+        "%" ASN_PRIdMAX, intmax_min);
+    assert(len_pmax < (int)sizeof(positive_max));
+    assert(len_nmin < (int)sizeof(negative_min));
+
+    enum asn_strtox_result_e result;
+    intmax_t value;
+
+    /*
+     * Test edge values first.
+     */
+    // Positive.
+    const char *last_pmax = &positive_max[len_pmax];
+    result = asn_strtoimax_lim(positive_max, &last_pmax, &value);
+    assert(result == ASN_STRTOX_OK);
+    assert(last_pmax == &positive_max[len_pmax]);
+    assert(value == intmax_max);
+    // Negative.
+    const char *last_nmin = &negative_min[len_nmin];
+    result = asn_strtoimax_lim(negative_min, &last_nmin, &value);
+    assert(result == ASN_STRTOX_OK);
+    assert(last_nmin == &negative_min[len_nmin]);
+    assert(value == intmax_min);
+
+    /*
+     * Test one smaller than edge evalues.
+     */
+    positive_max[len_pmax - 1]--;
+    negative_min[len_nmin - 1]--;
+    // Positive.
+    result = asn_strtoimax_lim(positive_max, &last_pmax, &value);
+    assert(result == ASN_STRTOX_OK);
+    assert(last_pmax == &positive_max[len_pmax]);
+    assert(value == intmax_max - 1);
+    // Negative.
+    result = asn_strtoimax_lim(negative_min, &last_nmin, &value);
+    assert(result == ASN_STRTOX_OK);
+    assert(last_nmin == &negative_min[len_nmin]);
+    assert(value == intmax_min + 1);
+
+    /*
+     * Test one bigger than edge evalues.
+     */
+    positive_max[len_pmax - 1] += 2;
+    negative_min[len_nmin - 1] += 2;
+    // Positive.
+    value = 42;
+    result = asn_strtoimax_lim(positive_max, &last_pmax, &value);
+    assert(result == ASN_STRTOX_ERROR_RANGE);
+    assert(last_pmax == &positive_max[len_pmax - 1]);
+    assert(value == 42);
+    // Negative.
+    value = 42;
+    result = asn_strtoimax_lim(negative_min, &last_nmin, &value);
+    assert(result == ASN_STRTOX_ERROR_RANGE);
+    assert(last_nmin == &negative_min[len_nmin - 1]);
+    assert(value == 42);
+
+    /*
+     * Get back to the edge.
+     * Append an extra digit at the end.
+     */
+    positive_max[len_pmax - 1]--;
+    negative_min[len_nmin - 1]--;
+    assert(len_pmax < (int)sizeof(positive_max) - 1);
+    assert(len_nmin < (int)sizeof(negative_min) - 1);
+    strcat(positive_max, "0");
+    strcat(negative_min, "0");
+    last_pmax++;
+    last_nmin++;
+    value = 42;
+    result = asn_strtoimax_lim(positive_max, &last_pmax, &value);
+    assert(result == ASN_STRTOX_OK);
+    assert(value == intmax_max);
+    result = asn_strtoimax_lim(negative_min, &last_nmin, &value);
+    assert(result == ASN_STRTOX_OK);
+    assert(value == intmax_min);
+}
+
+/*
+ * Check that asn_strtoimax_lim() always reaches the end of the numeric
+ * sequence, even if it can't fit into the range.
+ */
+static void
+check_strtoimax_span() {
+    const intmax_t intmax_max = ((~(uintmax_t)0) >> 1);
+    const intmax_t almost_min = -((intmax_t)(intmax_max - 10));
+    char buf[64];
+    intmax_t value = 42;
+    enum asn_strtox_result_e result;
+
+    // Check a particular way to integer overflow.
+    // Check that we scan until the very end.
+    int len = snprintf(buf, sizeof(buf), "%" PRIdMAX "0</end>", almost_min);
+    assert(len < (int)sizeof(buf));
+    const char *nmlast = &buf[len];
+    result = asn_strtoimax_lim(buf, &nmlast, &value);
+    assert(*nmlast == '0');
+    assert((ptrdiff_t)(nmlast - buf) == (ptrdiff_t)(len - strlen("0</end>")));
+    assert(result == ASN_STRTOX_ERROR_RANGE);
+    assert(value == 42);
+
+    // Check a particular way to integer overflow.
+    // Check that we scan until the very end.
+    len = snprintf(buf, sizeof(buf), "%" PRIdMAX "</end>", almost_min);
+    assert(len < (int)sizeof(buf));
+    nmlast = &buf[len];
+    result = asn_strtoimax_lim(buf, &nmlast, &value);
+    assert(*nmlast == '<');
+    assert((ptrdiff_t)(nmlast - buf) == (ptrdiff_t)(len - strlen("</end>")));
+    assert(result == ASN_STRTOX_EXTRA_DATA);
+    assert(value == almost_min);
+
+}
+
 int
 main() {
 	uint8_t buf1[] = { 1 };
@@ -298,5 +421,8 @@ main() {
         }
 #endif
 
+    check_strtoimax();
+    check_strtoimax_span();
+
 	return 0;
 }