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

[Marcelo Cerri]

Perfect!

-- 
Regards,
Marcelo

On Tue, Oct 18, 2016 at 11:19:50PM +0100, Colin Ian King wrote:
> 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: 473 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20161019/547e7c08/attachment.sig>