linux-kernel - Re: [PATCH] sctp: fix a potential missing-check bug

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 3 May 2018 10:39:45 -0300
From:   Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To:     Wenwen Wang <wang6495@....edu>
Cc:     Kangjie Lu <kjlu@....edu>, Vlad Yasevich <vyasevich@...il.com>,
        Neil Horman <nhorman@...driver.com>,
        "David S. Miller" <davem@...emloft.net>,
        "open list:SCTP PROTOCOL" <linux-sctp@...r.kernel.org>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] sctp: fix a potential missing-check bug

On Thu, May 03, 2018 at 08:31:28AM -0500, Wenwen Wang wrote:
> On Thu, May 3, 2018 at 7:46 AM, Marcelo Ricardo Leitner
> <marcelo.leitner@...il.com> wrote:
> > On Thu, May 03, 2018 at 07:01:51AM -0500, Wenwen Wang wrote:
> >> On Wed, May 2, 2018 at 8:48 PM, Marcelo Ricardo Leitner
> >> <marcelo.leitner@...il.com> wrote:
> >> > On Wed, May 02, 2018 at 08:27:05PM -0500, Wenwen Wang wrote:
> >> >> On Wed, May 2, 2018 at 8:24 PM, Marcelo Ricardo Leitner
> >> >> <marcelo.leitner@...il.com> wrote:
> >> >> > On Wed, May 02, 2018 at 08:15:45PM -0500, Wenwen Wang wrote:
> >> >> >> In sctp_setsockopt_maxseg(), the integer 'val' is compared against min_len
> >> >> >> and max_len to check whether it is in the appropriate range. If it is not,
> >> >> >> an error code -EINVAL will be returned. This is enforced by a security
> >> >> >> check. But, this check is only executed when 'val' is not 0. In fact, if
> >> >> >> 'val' is 0, it will be assigned with a new value (if the return value of
> >> >> >> the function sctp_id2assoc() is not 0) in the following execution. However,
> >> >> >> this new value of 'val' is not checked before it is used to assigned to
> >> >> >> asoc->user_frag. That means it is possible that the new value of 'val'
> >> >> >> could be out of the expected range. This can cause security issues
> >> >> >> such as buffer overflows, e.g., the new value of 'val' is used as an index
> >> >> >> to access a buffer.
> >> >> >>
> >> >> >> This patch inserts a check for the new value of 'val' to see if it is in
> >> >> >> the expected range. If it is not, an error code -EINVAL will be returned.
> >> >> >>
> >> >> >> Signed-off-by: Wenwen Wang <wang6495@....edu>
> >> >> >> ---
> >> >> >>  net/sctp/socket.c | 22 +++++++++++-----------
> >> >> >>  1 file changed, 11 insertions(+), 11 deletions(-)
> >> >> >
> >> >> > ?
> >> >> > This patch is the same as previous one. git send-email <old file>
> >> >> > maybe?
> >> >> >
> >> >> >   Marcelo
> >> >>
> >> >> Thanks for your suggestion, Marcelo. I can send the old file. But, I
> >> >> have added a line of comment in this patch.
> >> >
> >> > I meant if you had sent the old patch again by accident, because you
> >> > said you worked on an old version of the tree, but then posted a patch
> >> > that also doesn't use the new MTU function I mentioned.
> >> >
> >> >   Marcelo
> >>
> >> I worked on the latest kernel. But, I didn't find the MTU function
> >> sctp_mtu_payload().
> >
> > Which tree are you using?
> > [a] git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
> >    or
> > [b] git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
> > ?
> >
> > The function isn't on [a] yet, but it is on [b].
> >
> >   Marcelo
>
> Many thanks for your patience, Marcelo :)
>
> The tree I am working on is:
> git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

Ahh! That explains the discrepancy :)
For networking patches, please refer to
Documentation/networking/netdev-FAQ.txt
It describes what the 2 trees I pointed out are and how they should be
used.
In short, both net and net-next are always newer than the one you're
using for networking subsystem.

Regards,
Marcelo