Opened 9 years ago

Closed 8 years ago

#5316 closed Bug (fixed)

reads of uninitialized memory in quark.c/tr-getopt.c

Reported by: gsauthof Owned by: jordan
Priority: Normal Milestone: 2.80
Component: libtransmission Version: 2.77
Severity: Normal Keywords:
Cc:

Description

Used current trunk. See following trivial patches:

--- a/libtransmission/tr-getopt.c	Sat Mar 02 17:34:42 2013 +0100
+++ b/libtransmission/tr-getopt.c	Sat Mar 02 17:43:00 2013 +0100
@@ -150,7 +150,8 @@
         size_t len = o->longName ? strlen (o->longName) : 0;
 
         if ((matchlen < len) && !memcmp (str, "--", 2)
-          && !memcmp (str + 2, o->longName, len)
+          // don't read unitialized memory after the end of str
+          && !strcmp (str + 2, o->longName)
           && (str[len + 2] == '\0' || (o->has_arg && str[len + 2] == '=')))
         {
             matchlen = len;
@@ -161,8 +162,9 @@
         len = o->shortName ? strlen (o->shortName) : 0;
 
         if ((matchlen < len) && !memcmp (str, "-", 1)
-          && !memcmp (str + 1, o->shortName, len)
-          && (str[len + 1] == '\0' || o->has_arg))
+          && (str[len + 1] == '\0' || o->has_arg)
+          // don't read unitialized memory after the end of str
+          && !strcmp (str + 1, o->shortName))
         {
             matchlen = len;
             match = o;


And:

--- a/libtransmission/quark.c	Sat Mar 02 17:43:25 2013 +0100
+++ b/libtransmission/quark.c	Sat Mar 02 17:54:58 2013 +0100
@@ -399,12 +399,7 @@
   const struct tr_key_struct * a = va;
   const struct tr_key_struct * b = vb;
 
-  ret = memcmp (a->str, b->str, a->len);
-
-  if (!ret && (a->len != b->len))
-    ret = a->len < b->len ? -1 : 1;
-
-  return ret;
+  return strncmp (a->str, b->str, a->len);
 }

Change History (4)

comment:1 Changed 8 years ago by jordan

  • Milestone None Set deleted
  • Status changed from new to assigned

The first two fixes look right, but that one in quark breaks the regression tests. IMO that one could be fixed by replacing the third argument to memcmp() with MIN(a->len, b->len).

Thanks for the patch!

comment:2 Changed 8 years ago by jordan

Actually the first patch isn't right either because there could be the '=' argument after the key, so we can't just use strcmp().

comment:3 Changed 8 years ago by jordan

r14066: add unit tests for tr-getopt

comment:4 Changed 8 years ago by jordan

  • Milestone set to 2.80
  • Resolution set to fixed
  • Status changed from assigned to closed
  • Version changed from 2.77+ to 2.77

r14067: replace the memcmp() code that caused the UMR errors.

Reassigning version/milestone since the tr-getopt errors are in released code, not just nightly 2.77+ code.

Note: See TracTickets for help on using tickets.