lkml.org 
[lkml]   [2018]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 263/366] KEYS: DNS: fix parsing multiple options
    3.16.61-rc1 review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Eric Biggers <ebiggers@google.com>

    commit c604cb767049b78b3075497b80ebb8fd530ea2cc upstream.

    My recent fix for dns_resolver_preparse() printing very long strings was
    incomplete, as shown by syzbot which still managed to hit the
    WARN_ONCE() in set_precision() by adding a crafted "dns_resolver" key:

    precision 50001 too large
    WARNING: CPU: 7 PID: 864 at lib/vsprintf.c:2164 vsnprintf+0x48a/0x5a0

    The bug this time isn't just a printing bug, but also a logical error
    when multiple options ("#"-separated strings) are given in the key
    payload. Specifically, when separating an option string into name and
    value, if there is no value then the name is incorrectly considered to
    end at the end of the key payload, rather than the end of the current
    option. This bypasses validation of the option length, and also means
    that specifying multiple options is broken -- which presumably has gone
    unnoticed as there is currently only one valid option anyway.

    A similar problem also applied to option values, as the kstrtoul() when
    parsing the "dnserror" option will read past the end of the current
    option and into the next option.

    Fix these bugs by correctly computing the length of the option name and
    by copying the option value, null-terminated, into a temporary buffer.

    Reproducer for the WARN_ONCE() that syzbot hit:

    perl -e 'print "#A#", "\0" x 50000' | keyctl padd dns_resolver desc @s

    Reproducer for "dnserror" option being parsed incorrectly (expected
    behavior is to fail when seeing the unknown option "foo", actual
    behavior was to read the dnserror value as "1#foo" and fail there):

    perl -e 'print "#dnserror=1#foo\0"' | keyctl padd dns_resolver desc @s

    Reported-by: syzbot <syzkaller@googlegroups.com>
    Fixes: 4a2d789267e0 ("DNS: If the DNS server returns an error, allow that to be cached [ver #2]")
    Signed-off-by: Eric Biggers <ebiggers@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    net/dns_resolver/dns_key.c | 28 ++++++++++++++++------------
    1 file changed, 16 insertions(+), 12 deletions(-)

    --- a/net/dns_resolver/dns_key.c
    +++ b/net/dns_resolver/dns_key.c
    @@ -89,35 +89,39 @@ dns_resolver_instantiate(struct key *key
    opt++;
    kdebug("options: '%s'", opt);
    do {
    + int opt_len, opt_nlen;
    const char *eq;
    - int opt_len, opt_nlen, opt_vlen, tmp;
    + char optval[128];

    next_opt = memchr(opt, '#', end - opt) ?: end;
    opt_len = next_opt - opt;
    - if (opt_len <= 0 || opt_len > 128) {
    + if (opt_len <= 0 || opt_len > sizeof(optval)) {
    pr_warn_ratelimited("Invalid option length (%d) for dns_resolver key\n",
    opt_len);
    return -EINVAL;
    }

    - eq = memchr(opt, '=', opt_len) ?: end;
    - opt_nlen = eq - opt;
    - eq++;
    - opt_vlen = next_opt - eq; /* will be -1 if no value */
    -
    - tmp = opt_vlen >= 0 ? opt_vlen : 0;
    - kdebug("option '%*.*s' val '%*.*s'",
    - opt_nlen, opt_nlen, opt, tmp, tmp, eq);
    + eq = memchr(opt, '=', opt_len);
    + if (eq) {
    + opt_nlen = eq - opt;
    + eq++;
    + memcpy(optval, eq, next_opt - eq);
    + optval[next_opt - eq] = '\0';
    + } else {
    + opt_nlen = opt_len;
    + optval[0] = '\0';
    + }
    +
    + kdebug("option '%*.*s' val '%s'",
    + opt_nlen, opt_nlen, opt, optval);

    /* see if it's an error number representing a DNS error
    * that's to be recorded as the result in this key */
    if (opt_nlen == sizeof(DNS_ERRORNO_OPTION) - 1 &&
    memcmp(opt, DNS_ERRORNO_OPTION, opt_nlen) == 0) {
    kdebug("dns error number option");
    - if (opt_vlen <= 0)
    - goto bad_option_value;

    - ret = kstrtoul(eq, 10, &derrno);
    + ret = kstrtoul(optval, 10, &derrno);
    if (ret < 0)
    goto bad_option_value;

    \
     
     \ /
      Last update: 2018-11-11 21:25    [W:4.169 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site