[alsa-devel] [PATCH] alsactl: sprintf to snprintf prevent buffer overflow

Doug Goldstein cardoe at cardoe.com
Wed May 1 18:30:26 CEST 2013


sprintf() is a bit dangerous unless you explicitly know your type size
and want to keep it in sync always. Its safer to just use snprintf() and
ensure your string doesn't overflow and is NULL terminated.

Signed-off-by: Doug Goldstein <cardoe at cardoe.com>
---
This was written to fix the buffer overflow crash in alsactl with the
1.0.27 release, which is documented in Gentoo bug #468160 [1]. The
issue is actually already fixed upstream by
95788fea25c1a59985828d4b91af0772d077600b however the fix just changes
the size and continues to use sprintf() which IMHO is still dangerous.
The attached patch will fix the issue in a safer manner for good.

[1] https://bugs.gentoo.org/show_bug.cgi?id=468160
---
 alsactl/lock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/alsactl/lock.c b/alsactl/lock.c
index fce208b..587a109 100644
--- a/alsactl/lock.c
+++ b/alsactl/lock.c
@@ -53,9 +53,9 @@ static int state_lock_(const char *file, int lock, int timeout)
 	lck.l_len = 11;
 	lck.l_pid = 0;
 	if (lock) {
-		sprintf(lcktxt, "%10li\n", (long)getpid());
+		snprintf(lcktxt, sizeof(lcktxt), "%10li\n", (long)getpid());
 	} else {
-		sprintf(lcktxt, "%10s\n", "");
+		snprintf(lcktxt, sizeof(lcktxt), "%10s\n", "");
 	}
 	while (fd < 0 && timeout-- > 0) {
 		fd = open(nfile, O_RDWR);
-- 
1.8.2.1



More information about the Alsa-devel mailing list