lkml.org 
[lkml]   [2020]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: UBSAN: array-index-out-of-bounds in alg_bind
On Fri, Oct 16, 2020 at 01:12:24AM -0700, syzbot wrote:
> dashboard link: https://syzkaller.appspot.com/bug?extid=92ead4eb8e26a26d465e
> [...]
> Reported-by: syzbot+92ead4eb8e26a26d465e@syzkaller.appspotmail.com
> [...]
> UBSAN: array-index-out-of-bounds in crypto/af_alg.c:166:2
> index 91 is out of range for type '__u8 [64]'

This seems to be an "as intended", if very odd. false positive (the actual
memory area is backed by the on-stack _K_SS_MAXSIZE-sized sockaddr_storage
"address" variable in __sys_bind. But yes, af_alg's salg_name member
size here doesn't make sense. The origin appears to be that 3f69cc60768b
("crypto: af_alg - Allow arbitrarily long algorithm names") intentionally
didn't extend the kernel structure (which is actually just using the UAPI
structure). I don't see a reason the UAPI couldn't have been extended:
it's a sockaddr implementation, so the size is always passed in as part
of the existing API.

At the very least the kernel needs to switch to using a correctly-sized
structure: I expected UBSAN_BOUNDS to be enabled globally by default at
some point in the future (with the minimal runtime -- the
instrumentation is tiny and catches real issues).

Reproduction:

struct sockaddr_alg sa = {
.salg_family = AF_ALG,
.salg_type = "skcipher",
.salg_name = "cbc(aes)"
};
fd = socket(AF_ALG, SOCK_SEQPACKET, 0);
bind(fd, (void *)&sa, sizeof(sa));

Replace "sizeof(sa)" with x where 64<x<=128.

--
Kees Cook

\
 
 \ /
  Last update: 2020-10-17 08:03    [W:0.101 / U:1.412 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site