Opened 10 years ago
Closed 10 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 10 years ago by jordan
- Milestone None Set deleted
- Status changed from new to assigned
comment:2 Changed 10 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 10 years ago by jordan
r14066: add unit tests for tr-getopt
comment:4 Changed 10 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.
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!