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 for Android: free password hash cracker in your pocket

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

Date:	Thu, 5 Aug 2010 21:59:52 +0800
From:	Changli Gao <xiaosuo@...il.com>
To:	Dan Carpenter <error27@...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

On Thu, Aug 5, 2010 at 9:55 PM, Dan Carpenter <error27@...il.com> wrote:
> 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.
>

Thanks, Dan. I got it.

-- 
Regards,
Changli Gao(xiaosuo@...il.com)
--
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