linux-kernel - strncat() misuse (was: Re: dm_attr_{name,uuid}_show buffer overflow? (was: Re: linux-next: Tree for January 5))

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:	Mon, 5 Jan 2009 23:18:38 +0100 (CET)
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Stephen Rothwell <sfr@...b.auug.org.au>,
	Milan Broz <mbroz@...hat.com>,
	Alasdair G Kergon <agk@...hat.com>,
	Jaya Kumar <jayakumar.video@...il.com>,
	Laurent Pinchart <laurent.pinchart@...net.be>,
	Mauro Carvalho Chehab <mchehab@...radead.org>,
	Gene Sally <Gene.Sally@...esys.com>,
	Sam Ravnborg <sam@...nborg.org>,
	Andrew Morton <akpm@...ux-foundation.org>
cc:	linux-next@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: strncat() misuse (was: Re: dm_attr_{name,uuid}_show buffer overflow?
 (was: Re: linux-next: Tree for January 5))

On Mon, 5 Jan 2009, Geert Uytterhoeven wrote:
> On Mon, 5 Jan 2009, Stephen Rothwell wrote:
> > Status of my local build tests will be at
> > http://kisskb.ellerman.id.au/linux-next .  If maintainers want to give
> > advice about cross compilers/configs that work, we are always open to add
> > more builds.
> 
> On m68k, you get http://kisskb.ellerman.id.au/kisskb/buildresult/65466/:
> 
> | ERROR: "strlen" [drivers/md/dm-mod.ko] undefined!
> 
> which triggered my attention.
> 
> drivers/md/dm-sysfs.c:
> 
> | static ssize_t dm_attr_name_show(struct mapped_device *md, char *buf)
> | {
> |         if (dm_copy_name_and_uuid(md, buf, NULL))
> |                 return -EIO;
> | 
> |         strncat(buf, "\n", DM_NAME_LEN);
> |         return strnlen(buf, DM_NAME_LEN);
> | }

    [...]

> And I think that's the real bug: `strncat(buf, "\n", DM_NAME_LEN);' copies at
> most DM_NAME_LEN characters from source "\n", which has the known size 2,
> which is always smaller than DM_NAME_LEN. Hence the check is optimized away,
> and the "\n" is _always_ appended!
> 
> Probably the intention was to limit the string in _buf_ (not the source string
> "\n") to DM_NAME_LEN? If yes, this may cause a buffer overflow.
> 
> The same bug is present in dm_attr_uuid_show().

A quick `git grep strncat' shows a few more misusers of strncat() that may
cause buffer overflows and should be converted to strlcat():

| drivers/media/video/usbvideo/konicawc.c:	strncat(cam->input_physname, "/input0", sizeof(cam->input_physname));
| drivers/media/video/usbvideo/quickcam_messenger.c:	strncat(cam->input_physname, "/input0", sizeof(cam->input_physname));
| drivers/media/video/uvc/uvc_driver.c:		strncat(format->name, buffer[8] & (1 << 7) ? " 60Hz" : " 50Hz",
| usr/gen_init_cpio.c:                       strncat(expanded, getenv(env_var), PATH_MAX);
| usr/gen_init_cpio.c:                       strncat(expanded, end + 1, PATH_MAX);

Let's hope I didn't miss any. Good night! ;-)

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/