Commit d4285f03 authored by Nick Mathewson's avatar Nick Mathewson 🦀
Browse files

Extend tor_sscanf so it can replace sscanf in rephist.c

Fixes bug 4195 and Coverity CID 448
parent e12eba55
Loading
Loading
Loading
Loading

changes/bug4195

0 → 100644
+6 −0
Original line number Diff line number Diff line
  o Minor features:
    - Enhance our internal sscanf replacement so that we can eliminate
      the last remaining uses of the system sscanf. (Though those uses
      of sscanf were safe, sscanf itself is generally error prone, so
      we want to eliminate when we can.) Fixes ticket 4195 and Coverity
      CID 448.
+142 −9
Original line number Diff line number Diff line
@@ -2694,9 +2694,9 @@ digit_to_num(char d)
 * success, store the result in <b>out</b>, advance bufp to the next
 * character, and return 0.  On failure, return -1. */
static int
scan_unsigned(const char **bufp, unsigned *out, int width, int base)
scan_unsigned(const char **bufp, unsigned long *out, int width, int base)
{
  unsigned result = 0;
  unsigned long result = 0;
  int scanned_so_far = 0;
  const int hex = base==16;
  tor_assert(base == 10 || base == 16);
@@ -2708,8 +2708,8 @@ scan_unsigned(const char **bufp, unsigned *out, int width, int base)
  while (**bufp && (hex?TOR_ISXDIGIT(**bufp):TOR_ISDIGIT(**bufp))
         && scanned_so_far < width) {
    int digit = hex?hex_decode_digit(*(*bufp)++):digit_to_num(*(*bufp)++);
    unsigned new_result = result * base + digit;
    if (new_result > UINT32_MAX || new_result < result)
    unsigned long new_result = result * base + digit;
    if (new_result < result)
      return -1; /* over/underflow. */
    result = new_result;
    ++scanned_so_far;
@@ -2722,6 +2722,89 @@ scan_unsigned(const char **bufp, unsigned *out, int width, int base)
  return 0;
}

/** Helper: Read an signed int from *<b>bufp</b> of up to <b>width</b>
 * characters.  (Handle arbitrary width if <b>width</b> is less than 0.)  On
 * success, store the result in <b>out</b>, advance bufp to the next
 * character, and return 0.  On failure, return -1. */
static int
scan_signed(const char **bufp, long *out, int width)
{
  int neg = 0;
  unsigned long result = 0;

  if (!bufp || !*bufp || !out)
    return -1;
  if (width<0)
    width=MAX_SCANF_WIDTH;

  if (**bufp == '-') {
    neg = 1;
    ++*bufp;
    --width;
  }

  if (scan_unsigned(bufp, &result, width, 10) < 0)
    return -1;

  if (neg) {
    if (result > ((unsigned long)LONG_MAX) + 1)
      return -1; /* Underflow */
    *out = -(long)result;
  } else {
    if (result > LONG_MAX)
      return -1; /* Overflow */
    *out = (long)result;
  }

  return 0;
}

/** Helper: Read a decimal-formatted double from *<b>bufp</b> of up to
 * <b>width</b> characters.  (Handle arbitrary width if <b>width</b> is less
 * than 0.)  On success, store the result in <b>out</b>, advance bufp to the
 * next character, and return 0.  On failure, return -1. */
static int
scan_double(const char **bufp, double *out, int width)
{
  int neg = 0;
  double result = 0;
  int scanned_so_far = 0;

  if (!bufp || !*bufp || !out)
    return -1;
  if (width<0)
    width=MAX_SCANF_WIDTH;

  if (**bufp == '-') {
    neg = 1;
    ++*bufp;
  }

  while (**bufp && TOR_ISDIGIT(**bufp) && scanned_so_far < width) {
    const int digit = digit_to_num(*(*bufp)++);
    result = result * 10 + digit;
    ++scanned_so_far;
  }
  if (**bufp == '.') {
    double fracval = 0, denominator = 1;
    ++*bufp;
    ++scanned_so_far;
    while (**bufp && TOR_ISDIGIT(**bufp) && scanned_so_far < width) {
      const int digit = digit_to_num(*(*bufp)++);
      fracval = fracval * 10 + digit;
      denominator *= 10;
      ++scanned_so_far;
    }
    result += fracval / denominator;
  }

  if (!scanned_so_far) /* No actual digits scanned */
    return -1;

  *out = neg ? -result : result;
  return 0;
}

/** Helper: copy up to <b>width</b> non-space characters from <b>bufp</b> to
 * <b>out</b>.  Make sure <b>out</b> is nul-terminated. Advance <b>bufp</b>
 * to the next non-space character or the EOS. */
@@ -2758,6 +2841,7 @@ tor_vsscanf(const char *buf, const char *pattern, va_list ap)
      }
    } else {
      int width = -1;
      int longmod = 0;
      ++pattern;
      if (TOR_ISDIGIT(*pattern)) {
        width = digit_to_num(*pattern++);
@@ -2770,17 +2854,57 @@ tor_vsscanf(const char *buf, const char *pattern, va_list ap)
        if (!width) /* No zero-width things. */
          return -1;
      }
      if (*pattern == 'l') {
        longmod = 1;
        ++pattern;
      }
      if (*pattern == 'u' || *pattern == 'x') {
        unsigned *u = va_arg(ap, unsigned *);
        unsigned long u;
        const int base = (*pattern == 'u') ? 10 : 16;
        if (!*buf)
          return n_matched;
        if (scan_unsigned(&buf, u, width, base)<0)
        if (scan_unsigned(&buf, &u, width, base)<0)
          return n_matched;
        if (longmod) {
          unsigned long *out = va_arg(ap, unsigned long *);
          *out = u;
        } else {
          unsigned *out = va_arg(ap, unsigned *);
          if (u > UINT_MAX)
            return n_matched;
          *out = u;
        }
        ++pattern;
        ++n_matched;
      } else if (*pattern == 'f') {
        double *d = va_arg(ap, double *);
        if (!longmod)
          return -1; /* float not supported */
        if (!*buf)
          return n_matched;
        if (scan_double(&buf, d, width)<0)
          return n_matched;
        ++pattern;
        ++n_matched;
      } else if (*pattern == 'd') {
        long lng=0;
        if (scan_signed(&buf, &lng, width)<0)
          return n_matched;
        if (longmod) {
          long *out = va_arg(ap, long *);
          *out = lng;
        } else {
          int *out = va_arg(ap, int *);
          if (lng < INT_MIN || lng > INT_MAX)
            return n_matched;
          *out = (int)lng;
        }
        ++pattern;
        ++n_matched;
      } else if (*pattern == 's') {
        char *s = va_arg(ap, char *);
        if (longmod)
          return -1;
        if (width < 0)
          return -1;
        if (scan_string(&buf, s, width)<0)
@@ -2789,6 +2913,8 @@ tor_vsscanf(const char *buf, const char *pattern, va_list ap)
        ++n_matched;
      } else if (*pattern == 'c') {
        char *ch = va_arg(ap, char *);
        if (longmod)
          return -1;
        if (width != -1)
          return -1;
        if (!*buf)
@@ -2799,6 +2925,8 @@ tor_vsscanf(const char *buf, const char *pattern, va_list ap)
      } else if (*pattern == '%') {
        if (*buf != '%')
          return n_matched;
        if (longmod)
          return -1;
        ++buf;
        ++pattern;
      } else {
@@ -2812,9 +2940,14 @@ tor_vsscanf(const char *buf, const char *pattern, va_list ap)

/** Minimal sscanf replacement: parse <b>buf</b> according to <b>pattern</b>
 * and store the results in the corresponding argument fields.  Differs from
 * sscanf in that it: Only handles %u, %x, %c and %Ns.  Does not handle
 * arbitrarily long widths. %u and %x do not consume any space.  Is
 * locale-independent.  Returns -1 on malformed patterns.
 * sscanf in that:
 * <ul><li>It only handles %u, %lu, %x, %lx, %<NUM>s, %d, %ld, %lf, and %c.
 *     <li>It only handles decimal inputs for %lf. (12.3, not 1.23e1)
 *     <li>It does not handle arbitrarily long widths.
 *     <li>Numbers do not consume any space characters.
 *     <li>It is locale-independent.
 *     <li>%u and %x do not consume any space.
 *     <li>It returns -1 on malformed patterns.</ul>
 *
 * (As with other locale-independent functions, we need this to parse data that
 * is in ASCII without worrying that the C library's locale-handling will make
+3 −3
Original line number Diff line number Diff line
@@ -1136,7 +1136,7 @@ rep_hist_load_mtbf_data(time_t now)
    wfu_timebuf[0] = '\0';

    if (format == 1) {
      n = sscanf(line, "%40s %ld %lf S=%10s %8s",
      n = tor_sscanf(line, "%40s %ld %lf S=%10s %8s",
                 hexbuf, &wrl, &trw, mtbf_timebuf, mtbf_timebuf+11);
      if (n != 3 && n != 5) {
        log_warn(LD_HIST, "Couldn't scan line %s", escaped(line));
@@ -1153,7 +1153,7 @@ rep_hist_load_mtbf_data(time_t now)
      wfu_idx = find_next_with(lines, i+1, "+WFU ");
      if (mtbf_idx >= 0) {
        const char *mtbfline = smartlist_get(lines, mtbf_idx);
        n = sscanf(mtbfline, "+MTBF %lu %lf S=%10s %8s",
        n = tor_sscanf(mtbfline, "+MTBF %lu %lf S=%10s %8s",
                   &wrl, &trw, mtbf_timebuf, mtbf_timebuf+11);
        if (n == 2 || n == 4) {
          have_mtbf = 1;
@@ -1164,7 +1164,7 @@ rep_hist_load_mtbf_data(time_t now)
      }
      if (wfu_idx >= 0) {
        const char *wfuline = smartlist_get(lines, wfu_idx);
        n = sscanf(wfuline, "+WFU %lu %lu S=%10s %8s",
        n = tor_sscanf(wfuline, "+WFU %lu %lu S=%10s %8s",
                   &wt_uptime, &total_wt_time,
                   wfu_timebuf, wfu_timebuf+11);
        if (n == 2 || n == 4) {
+75 −0
Original line number Diff line number Diff line
@@ -1461,12 +1461,28 @@ test_util_control_formats(void)
  tor_free(out);
}

#define test_feq(value1,value2) do {                               \
    double v1 = (value1), v2=(value2);                             \
    double tf_diff = v1-v2;                                        \
    double tf_tolerance = ((v1+v2)/2.0)/1e8;                       \
    if (tf_diff<0) tf_diff=-tf_diff;                               \
    if (tf_tolerance<0) tf_tolerance=-tf_tolerance;                \
    if (tf_diff<tf_tolerance) {                                    \
      TT_BLATHER(("%s ~~ %s: %f ~~ %f",#value1,#value2,v1,v2));  \
    } else {                                                       \
      TT_FAIL(("%s ~~ %s: %f != %f",#value1,#value2,v1,v2)); \
    }                                                              \
  } while(0)

static void
test_util_sscanf(void)
{
  unsigned u1, u2, u3;
  char s1[20], s2[10], s3[10], ch;
  int r;
  long lng1,lng2;
  int int1, int2;
  double d1,d2,d3,d4;

  /* Simple tests (malformed patterns, literal matching, ...) */
  test_eq(-1, tor_sscanf("123", "%i", &r)); /* %i is not supported */
@@ -1595,6 +1611,65 @@ test_util_sscanf(void)
  test_eq(4, tor_sscanf("1.2.3 foobar", "%u.%u.%u%c", &u1, &u2, &u3, &ch));
  test_eq(' ', ch);

  r = tor_sscanf("12345 -67890 -1", "%d %ld %d", &int1, &lng1, &int2);
  test_eq(r,3);
  test_eq(int1, 12345);
  test_eq(lng1, -67890);
  test_eq(int2, -1);

#if SIZEOF_INT == 4
  r = tor_sscanf("-2147483648. 2147483647.", "%d. %d.", &int1, &int2);
  test_eq(r,2);
  test_eq(int1, -2147483648);
  test_eq(int2, 2147483647);

  r = tor_sscanf("-2147483679.", "%d.", &int1);
  test_eq(r,0);

  r = tor_sscanf("2147483678.", "%d.", &int1);
  test_eq(r,0);
#elif SIZEOF_INT == 8
  r = tor_sscanf("-9223372036854775808. 9223372036854775807.",
                 "%d. %d.", &int1, &int2);
  test_eq(r,2);
  test_eq(int1, -9223372036854775808);
  test_eq(int2, 9223372036854775807);

  r = tor_sscanf("-9223372036854775809.", "%d.", &int1);
  test_eq(r,0);

  r = tor_sscanf("9223372036854775808.", "%d.", &int1);
  test_eq(r,0);
#endif

#if SIZEOF_LONG == 4
  r = tor_sscanf("-2147483648. 2147483647.", "%ld. %ld.", &lng1, &lng2)
  test_eq(r,2);
  test_eq(lng1, -2147483647 - 1);
  test_eq(lng2, 2147483647);
#elif SIZEOF_LONG == 8
  r = tor_sscanf("-9223372036854775808. 9223372036854775807.",
                 "%ld. %ld.", &lng1, &lng2);
  test_eq(r,2);
  test_eq(lng1, -9223372036854775807L - 1);
  test_eq(lng2, 9223372036854775807L);

  r = tor_sscanf("-9223372036854775808. 9223372036854775808.",
                 "%ld. %ld.", &lng1, &lng2);
  test_eq(r,1);
  r = tor_sscanf("-9223372036854775809. 9223372036854775808.",
                 "%ld. %ld.", &lng1, &lng2);
  test_eq(r,0);
#endif

  r = tor_sscanf("123.456 .000007 -900123123.2000787 00003.2",
                 "%lf %lf %lf %lf", &d1,&d2,&d3,&d4);
  test_eq(r,4);
  test_feq(d1, 123.456);
  test_feq(d2, .000007);
  test_feq(d3, -900123123.2000787);
  test_feq(d4, 3.2);

 done:
  ;
}