Re: [PATCH 3/4] security/selinux: decrement sizeof size in strncmp

[Valdis . Kletnieks]

On Fri, 13 Nov 2009 22:26:20 +0100, Julia Lawall said:
> On Fri, 13 Nov 2009, Valdis.Kletnieks@xxxxxx 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...)

Attachment: pgp00000.pgp
Description: PGP signature