lkml.org 
[lkml]   [2020]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 006/121] hdlc_ppp: add range checks in ppp_cp_parse_cr()
    Date
    From: Dan Carpenter <dan.carpenter@oracle.com>

    [ Upstream commit 66d42ed8b25b64eb63111a2b8582c5afc8bf1105 ]

    There are a couple bugs here:
    1) If opt[1] is zero then this results in a forever loop. If the value
    is less than 2 then it is invalid.
    2) It assumes that "len" is more than sizeof(valid_accm) or 6 which can
    result in memory corruption.

    In the case of LCP_OPTION_ACCM, then we should check "opt[1]" instead
    of "len" because, if "opt[1]" is less than sizeof(valid_accm) then
    "nak_len" gets out of sync and it can lead to memory corruption in the
    next iterations through the loop. In case of LCP_OPTION_MAGIC, the
    only valid value for opt[1] is 6, but the code is trying to log invalid
    data so we should only discard the data when "len" is less than 6
    because that leads to a read overflow.

    Reported-by: ChenNan Of Chaitin Security Research Lab <whutchennan@gmail.com>
    Fixes: e022c2f07ae5 ("WAN: new synchronous PPP implementation for generic HDLC.")
    Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
    Reviewed-by: Eric Dumazet <edumazet@google.com>
    Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/net/wan/hdlc_ppp.c | 16 +++++++++++-----
    1 file changed, 11 insertions(+), 5 deletions(-)

    --- a/drivers/net/wan/hdlc_ppp.c
    +++ b/drivers/net/wan/hdlc_ppp.c
    @@ -386,11 +386,8 @@ static void ppp_cp_parse_cr(struct net_d
    }

    for (opt = data; len; len -= opt[1], opt += opt[1]) {
    - if (len < 2 || len < opt[1]) {
    - dev->stats.rx_errors++;
    - kfree(out);
    - return; /* bad packet, drop silently */
    - }
    + if (len < 2 || opt[1] < 2 || len < opt[1])
    + goto err_out;

    if (pid == PID_LCP)
    switch (opt[0]) {
    @@ -398,6 +395,8 @@ static void ppp_cp_parse_cr(struct net_d
    continue; /* MRU always OK and > 1500 bytes? */

    case LCP_OPTION_ACCM: /* async control character map */
    + if (opt[1] < sizeof(valid_accm))
    + goto err_out;
    if (!memcmp(opt, valid_accm,
    sizeof(valid_accm)))
    continue;
    @@ -409,6 +408,8 @@ static void ppp_cp_parse_cr(struct net_d
    }
    break;
    case LCP_OPTION_MAGIC:
    + if (len < 6)
    + goto err_out;
    if (opt[1] != 6 || (!opt[2] && !opt[3] &&
    !opt[4] && !opt[5]))
    break; /* reject invalid magic number */
    @@ -427,6 +428,11 @@ static void ppp_cp_parse_cr(struct net_d
    ppp_cp_event(dev, pid, RCR_GOOD, CP_CONF_ACK, id, req_len, data);

    kfree(out);
    + return;
    +
    +err_out:
    + dev->stats.rx_errors++;
    + kfree(out);
    }

    static int ppp_rx(struct sk_buff *skb)

    \
     
     \ /
      Last update: 2020-09-29 13:08    [W:4.143 / U:0.112 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site