linux-kernel - snprintf, overlapping destination and source

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-next>] [day] [month] [year] [list]

Message-ID: <87d1ukr5pf.fsf@rasmusvillemoes.dk>
Date:	Sat, 05 Dec 2015 21:38:52 +0100
From:	Rasmus Villemoes <linux@...musvillemoes.dk>
To:	linux-kernel@...r.kernel.org
Cc:	Kees Cook <keescook@...omium.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Julia Lawall <julia.lawall@...6.fr>
Subject: snprintf, overlapping destination and source

I did a search for code doing

  s[n]printf(buf, "...", ..., buf, ...)

and found a few instances. They all do it with the format string
beginning with "%s" and buf being passed as the corresponding parameter
(obviously to append to the existing string). That works (AFAICT), both
with the current printf implementation and with the string()
modification which is now in -mm. It would obviously go horribly wrong
if anything, even non-specifiers, precede the "%s" in the format
string.

The question is, do we want to officially support this particular case of
overlapping src and dst? Or should we close our eyes and hope it will
continue to work [1] and that it won't cause a caffeine-deprived hacker
to accidentally think one could also prepend to a buffer by doing
sprintf(buf, "...%s", ..., buf)? I'm actually surprised gcc doesn't warn
about this.

[1] Not that I can immediately think of a sane way to implement snprintf
where it won't work, but you never know...

My coccinelle-fu isn't sufficient to find cases where one of the buf
instances is a more complicated expressions involving buf as a
subexpression, as in

  s[n]printf(buf, "...", ..., buf + 4, ...)

or

  s[n]printf(&buf[len], "...", ..., buf, ...)

which would presumably always be wrong. Julia?

Rasmus

The cases I've found are

./drivers/gpu/drm/amd/amdkfd/kfd_topology.c:613:53-54: s[n]printf, overlapping source and destination buffers
./drivers/gpu/drm/amd/amdkfd/kfd_topology.c:618:16-17: s[n]printf, overlapping source and destination buffers
./drivers/gpu/drm/amd/amdkfd/kfd_topology.c:488:58-59: s[n]printf, overlapping source and destination buffers
./drivers/input/joystick/analog.c:445:59-60: s[n]printf, overlapping source and destination buffers
./drivers/leds/led-class-flash.c:215:32-33: s[n]printf, overlapping source and destination buffers
./drivers/media/pci/zoran/videocodec.c:120:39-40: s[n]printf, overlapping source and destination buffers
./drivers/media/rc/ati_remote.c:875:47-48: s[n]printf, overlapping source and destination buffers
./drivers/net/wireless/ti/wlcore/boot.c:125:24-25: s[n]printf, overlapping source and destination buffers
./drivers/net/wireless/ti/wlcore/boot.c:128:37-38: s[n]printf, overlapping source and destination buffers
./drivers/usb/atm/usbatm.c:1341:46-47: s[n]printf, overlapping source and destination buffers
--
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/