Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 567608 - sys-auth/skey: out of bounds stack read
Summary: sys-auth/skey: out of bounds stack read
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Ulrich Müller
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-05 22:31 UTC by Hanno Böck
Modified: 2016-03-03 19:00 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments
fix out of bounds in skey (skey-fix-oob.diff,473 bytes, patch)
2015-12-05 22:31 UTC, Hanno Böck
Details | Diff
Patch for the patchset (0001-put.c-Avoid-out-of-bounds-stack-read.patch,4.75 KB, patch)
2015-12-06 15:57 UTC, Ulrich Müller
Details | Diff
14_all_extract.patch (14_all_extract.patch,953 bytes, patch)
2015-12-06 20:32 UTC, Ulrich Müller
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hanno Böck gentoo-dev 2015-12-05 22:31:19 UTC
Created attachment 418616 [details, diff]
fix out of bounds in skey

I discovered an out of bounds stack read in sys-auth/skey.

This is not in the upstream code, instead it was added (or let's say countermeasures were removed) in Gentoo's
01_all_gentoo.patch

This was added by taviso 2003 and at some point changed by ulm.
I will attach a patch on top of Gentoo's patched code. But we should probably change the 01_all_gentoo.patch.
Comment 1 Ulrich Müller gentoo-dev 2015-12-06 15:55:11 UTC
(In reply to Hanno Boeck from comment #0)
> This is not in the upstream code, instead it was added (or let's say
> countermeasures were removed) in Gentoo's
> 01_all_gentoo.patch
> 
> This was added by taviso 2003 and at some point changed by ulm.

Our S/Key version is based on the OpenBSD version, with updates taken from NetBSD. Looking at the NetBSD code shows that the problem is not homemade, but already exists in the upstream version:
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libskey/put.c?rev=1.13&content-type=text/x-cvsweb-markup

> I will attach a patch on top of Gentoo's patched code. But we should
> probably change the 01_all_gentoo.patch.

I agree.
Comment 2 Ulrich Müller gentoo-dev 2015-12-06 15:57:56 UTC
Created attachment 418674 [details, diff]
Patch for the patchset

Please test.
Comment 3 Ulrich Müller gentoo-dev 2015-12-06 16:21:59 UTC
Or maybe we should fix extract() instead. (The function starts with 4 assert statements but still manages to get things wrong ...)
Comment 4 Ulrich Müller gentoo-dev 2015-12-06 20:32:28 UTC
Created attachment 418682 [details, diff]
14_all_extract.patch

I believe there is a second out-of-bounds access in function etob(), line 2164 of put.c:

	if ((p & 3) != extract (b, 64, 2)) 

This will read from b[10] but b has only 9 elements. So let's fix extract(), instead of working around its breakage.

Can you please test if attached patch fixes the problem for you?
Comment 5 Ulrich Müller gentoo-dev 2015-12-12 09:26:25 UTC
I have committed this patch now (which fixes both extract() and insert()):
https://gitweb.gentoo.org/dev/ulm.git/tree/patchsets/skey/1.1.5/14_all_extract-insert.patch?id=d37ba903d8d0c9c3d7de8280b55229c23cebad18

Please test -r10.