netdev - Re: [PATCH v2 net] hdlc_ppp: add range checks in ppp_cp_parse

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20200910.130047.2058654522640662791.davem@davemloft.net>
Date:   Thu, 10 Sep 2020 13:00:47 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     dan.carpenter@...cle.com
Cc:     khc@...waw.pl, kuba@...nel.org, netdev@...r.kernel.org,
        security@...nel.org, whutchennan@...il.com, greg@...ah.com
Subject: Re: [PATCH v2 net] hdlc_ppp: add range checks in ppp_cp_parse_cr()

From: Dan Carpenter <dan.carpenter@...cle.com>
Date: Wed, 9 Sep 2020 12:46:48 +0300

> 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@...il.com>
> Fixes: e022c2f07ae5 ("WAN: new synchronous PPP implementation for generic HDLC.")
> Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com>
> Reviewed-by: Eric Dumazet <edumazet@...gle.com>
> Reviewed-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
> v2: check opt[1] < 6 instead of len < 6 for the LCP_OPTION_ACCM case.

Applied and queued up for -stable, thanks Dan.