netdev - Re: [patch] isdn: fix information leak

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]

Message-ID: <20100805134910.GJ9031@bicker>
Date:	Thu, 5 Aug 2010 15:55:11 +0200
From:	Dan Carpenter <error27@...il.com>
To:	Changli Gao <xiaosuo@...il.com>
Cc:	Karsten Keil <isdn@...ux-pingi.de>, netdev@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [patch] isdn: fix information leak

First of all, I'm happy that you're reviewing these patches.  A lot of
learner, often untested, patches go through kernel-janitors and the
entire purpose of the list is to check each other's work.

Here is what I meant by an information leak.  The original code did:
	spid = kmalloc(SCIOC_SPIDSIZE, GFP_KERNEL);
	...
	strcpy(spid, rcvmsg->msg_data.byte_array);
	...
	if (copy_to_user(data->dataptr, spid, SCIOC_SPIDSIZE)) {

When you allocate memory with kmalloc() it doesn't clear that memory.  It
could have anything in there including passwords etc.  If there is a
short string in "rcvmsg->msg_data.byte_array" like "123" then the
strcpy() puts that in the first 4 bytes of "spid", but the rest of the
buffer is still full of passwords.

The copy_to_user() sends it to the user, and normally the user only reads
as far as the terminator, but if they read past that then they would see
all the passwords etc that were saved there.

This function is in the ioctl() probably only root has open() permission
on the device so it's not a big deal because root can already find your
password.  But I didn't dig that far.  It's just simpler to zero out the
memory instead of worrying about if it really is exploitable or not.

Also it's quite possible that the strcpy can't overflow.  But it's weird
for the code to leave space for the terminator and then not use it.  In
the end, I decided to do the cautious thing and be done with it.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html