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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 13 Nov 2009 18:08:13 -0500
From:	Valdis.Kletnieks@...edu
To:	Julia Lawall <julia@...u.dk>
Cc:	Casey Schaufler <casey@...aufler-ca.com>,
	James Morris <jmorris@...ei.org>,
	"Serge E. Hallyn" <serue@...ibm.com>,
	Stephen Smalley <sds@...ho.nsa.gov>,
	Eric Paris <eparis@...isplace.org>,
	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH 3/4] security/selinux: decrement sizeof size in strncmp

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

Content of type "application/pgp-signature" skipped