From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Borkmann Subject: Re: [PATCH net] filter: prevent nla extensions to peek beyond the end of the message Date: Sun, 13 Apr 2014 19:34:45 +0200 Message-ID: <534ACAB5.5030500@redhat.com> References: <1397406213-15452-1-git-send-email-minipli@googlemail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: "David S. Miller" , netdev@vger.kernel.org, Patrick McHardy , Pablo Neira Ayuso To: Mathias Krause Return-path: Received: from mx1.redhat.com ([209.132.183.28]:57435 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754268AbaDMRe4 (ORCPT ); Sun, 13 Apr 2014 13:34:56 -0400 In-Reply-To: <1397406213-15452-1-git-send-email-minipli@googlemail.com> Sender: netdev-owner@vger.kernel.org List-ID: On 04/13/2014 06:23 PM, Mathias Krause wrote: > The BPF_S_ANC_NLATTR and BPF_S_ANC_NLATTR_NEST extensions fail to check > for a minimal message length before testing the supplied offset to be > within the bounds of the message. This allows the subtraction of the nla > header to underflow and therefore -- as the data type is unsigned -- > allowing far to big offset and length values for the search of the > netlink attribute. > > The remainder calculation for the BPF_S_ANC_NLATTR_NEST extension is > also wrong. It has the minuend und subtrahend mixed up, therefore > calculates a huge length value, allowing to overrun the end of the > message while looking for the netlink attribute. > > The following three BPF snippets will trigger the bugs when attached to > a UNIX datagram socket and parsing a message with length 1, 2 or 3. > > ,-[ PoC for missing size check in BPF_S_ANC_NLATTR ]-- > | ld #0x87654321 > | ldx #42 > | ld #nla > | ret a > `--- > > ,-[ PoC for the same bug in BPF_S_ANC_NLATTR_NEST ]-- > | ld #0x87654321 > | ldx #42 > | ld #nlan > | ret a > `--- > > ,-[ PoC for wrong remainder calculation in BPF_S_ANC_NLATTR_NEST ]-- > | ; (needs a fake netlink header at offset 0) > | ld #0 > | ldx #42 > | ld #nlan > | ret a > `--- > > Fix the first issue by ensuring the message length fulfills the minimal > size constrains of a nla header. Fix the second bug by getting the math > for the remainder calculation right. > > Fixes: 4738c1db15 ("[SKFILTER]: Add SKF_ADF_NLATTR instruction") > Fixes: d214c7537b ("filter: add SKF_AD_NLATTR_NEST to look for nested..") > Cc: Patrick McHardy > Cc: Pablo Neira Ayuso > Signed-off-by: Mathias Krause Acked-by: Daniel Borkmann