[PATCH][SRU][PRECISE] KEYS: ensure xbuf is large enough to fix buffer overflow in proc_keys_show (LP: #1634496)

[Colin Ian King]

On 18/10/16 20:35, Marcelo Cerri wrote:
> -- Regards, Marcelo On Tue, Oct 18, 2016 at 02:47:06PM +0100, Colin King
> wrote:
>> > From: Colin Ian King <colin.king at canonical.com>
>> > 
>> > BugLink: http://bugs.launchpad.net/bugs/1634496
>> > 
>> > CVE-2016-7042
>> > 
>> > When stack protect is enabled xbuf is too small for timeout data causing a
>> > buffer overflow and a stack protector corruption report.
>> > 
>> > OriginalAuthor: Vladis Dronov <vdronov at redhat.com>
>> > OriginalLocation: https://bugzilla.redhat.com/attachment.cgi?id=1200212&action=diff
>> > 
>> > Signed-off-by: Colin Ian King <colin.king at canonical.com>
>> > ---
>> >  security/keys/proc.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/security/keys/proc.c b/security/keys/proc.c
>> > index 49bbc97..3f7b410 100644
>> > --- a/security/keys/proc.c
>> > +++ b/security/keys/proc.c
>> > @@ -188,7 +188,7 @@ static int proc_keys_show(struct seq_file *m, void *v)
>> >  	struct timespec now;
>> >  	unsigned long timo;
>> >  	key_ref_t key_ref, skey_ref;
>> > -	char xbuf[12];
>> > +	char xbuf[16];
> Isn't safe to use the maximum value possible? 20 for the unsigned long
> max, +1 for the unit character and +1 for \0.
> 
#include <stdio.h>
#include <string.h>

int main()
{
        unsigned long timo = 1;
        char xbuf[64];
        size_t len, max = 0;

        do {
                timo = (timo << 1) | 1;
                if (timo < 60)
                        sprintf(xbuf, "%lus", timo);
                else if (timo < 60*60)
                        sprintf(xbuf, "%lum", timo / 60);
                else if (timo < 60*60*24)
                        sprintf(xbuf, "%luh", timo / (60*60));
                else if (timo < 60*60*24*7)
                        sprintf(xbuf, "%lud", timo / (60*60*24));
                else
                        sprintf(xbuf, "%luw", timo / (60*60*24*7));

                len = strlen(xbuf);
                if (len > max) {
                        max = len;
                        printf("%s %zu\n", xbuf, len);
                }
        } while (!(timo & 0x8000000000000000ULL));

        return max;
}

./a.out
3s 2
15s 3
110w 4
1775w 5
14202w 6
113623w 7
1817975w 8
14543804w 9
116350436w 10
1861606988w 11
14892855910w 12
119142847284w 13
1906285556558w 14
15250284452471w 15

So 15 is the longest string + 1 char = 16

>> >  	int rc;
>> >  
>> >  	key_ref = make_key_ref(key, 0);
>> > -- 
>> > 2.9.3
>> > 
>> > 
>> > -- 
>> > kernel-team mailing list
>> > kernel-team at lists.ubuntu.com
>> > https://lists.ubuntu.com/mailman/listinfo/kernel-team


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 837 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20161018/9b367da3/attachment.sig>