Messages in this thread | | | Date | Sat, 14 Nov 2009 16:22:44 +0100 (CET) | From | Julia Lawall <> | Subject | Re: [PATCH 3/4] security/selinux: decrement sizeof size in strncmp |
| |
On Fri, 13 Nov 2009, Valdis.Kletnieks@vt.edu wrote:
> On Fri, 13 Nov 2009 22:26:20 +0100, Julia Lawall said: > > On Fri, 13 Nov 2009, Valdis.Kletnieks@vt.edu wrote: > > > Julia, is there a way to use coccinelle to detect unsafe changes like that? Or > > > is expressing those semantics too difficult? > > > > Could you give a concrete example of something that would be a problem? > > If something like alias analysis is required, to know what strings a > > variable might be bound to, that might be difficult. Coccinelle works > > better when there is some concrete codeto match against. > > Here's a concrete example of how a previously audited strcmp() can go bad... > > struct foo { > char[16] a; /* old code allows 15 chars and 1 more for the \0 */ > int b; > int c; > } > > bzero(foo,sizeof(foo)); > > Now code can pretty safely mess with the first 15 bytes of foo->a and > we know we're OK if we call strcmp(foo->a,....) because that bzero() > nuked a[15] for us. It's safe to strncpy(foo->a,bar,15); and not worry > about the fact that if bar is 15 chars long, a trailing \0 won't be put in. > > Now somebody comes along and does: > > struct foo { > char *a; /* we need more than 15 chars for some oddball hardware */ > int b; > int c; > } > > bzero(foo,sizeof(foo)); > foo->a = kmalloc(32); /* whoops should have been kzmalloc */ > > Now suddenly, strncpy(foo->a,bar,31); *isn't* safe.... > > (Yes, I know there's plenty of blame to go around in this example - the failure > to use kzmalloc, the use of strncpy() without an explicit \0 being assigned > someplace, the use of strcmp() rather than strncmp()... But our tendency to > intentionally omit several steps of this to produce more efficient code means > it's easier to shoot ourselves in the foot...)
Thanks for the example. Coccinelle only finds patterns of code in one version, while this would require considering two versions at once. Such a thing could be interesting though.
julia
| |