File 0020-aplay-fix-buffer-overflow-and-tainted-form... of Package alsa-utils

Overview Repositories Revisions Requests Users Attributes Meta

File 0020-aplay-fix-buffer-overflow-and-tainted-format-string.patch of Package alsa-utils (Revision cc23c84ac72f466950072609a4001bb1)

Currently displaying revision cc23c84ac72f466950072609a4001bb1 , Show latest

From 4ce6a0a4af518700c3e44257af5f44ff24d58fc9 Mon Sep 17 00:00:00 2001
From: Mingjie Shen <shen497@purdue.edu>
Date: Wed, 6 Dec 2023 16:09:58 -0500
Subject: [PATCH] aplay: fix buffer overflow and tainted format string

Prior this commit, memcpy from names[0] to format[] will overwrite if
strlen(names[0]) is greater than 1024. Also, the length of malloc()ed
names[channel] is insufficient, leading to another buffer overwriting
when calling sprintf(). Moreover, the format string of sprintf()
can be controlled by user input. An attacker can exploit this weakness
to crash the program, disclose information or even execute arbitrary
code.

Fix by allocating enough space for arrays and using constant expressions
as the format strings.

Fixes: https://github.com/alsa-project/alsa-utils/pull/246/
Signed-off-by: Mingjie Shen <shen497@purdue.edu>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 aplay/aplay.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/aplay/aplay.c b/aplay/aplay.c
index 9cf36dee2d9d..f1c27b6c4929 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -3436,14 +3436,14 @@ static void playbackv(char **names, unsigned int count)
 
 	if (count == 1 && channels > 1) {
 		size_t len = strlen(names[0]);
-		char format[1024];
-		memcpy(format, names[0], len);
-		strcpy(format + len, ".%d");
-		len += 4;
+		char buf[len + 1];
+		strcpy(buf, names[0]);
+		/* 1 for "." + 3 for channel (<= 256) + 1 for null terminator */
+		len += 5;
 		names = malloc(sizeof(*names) * channels);
 		for (channel = 0; channel < channels; ++channel) {
 			names[channel] = malloc(len);
-			sprintf(names[channel], format, channel);
+			snprintf(names[channel], len, "%s.%d", buf, channel);
 		}
 		alloced = 1;
 	} else if (count != channels) {
@@ -3489,14 +3489,14 @@ static void capturev(char **names, unsigned int count)
 
 	if (count == 1) {
 		size_t len = strlen(names[0]);
-		char format[1024];
-		memcpy(format, names[0], len);
-		strcpy(format + len, ".%d");
-		len += 4;
+		char buf[len + 1];
+		strcpy(buf, names[0]);
+		/* 1 for "." + 3 for channel (<= 256) + 1 for null terminator */
+		len += 5;
 		names = malloc(sizeof(*names) * channels);
 		for (channel = 0; channel < channels; ++channel) {
 			names[channel] = malloc(len);
-			sprintf(names[channel], format, channel);
+			snprintf(names[channel], len, "%s.%d", buf, channel);
 		}
 		alloced = 1;
 	} else if (count != channels) {
-- 
2.35.3

Places

File 0020-aplay-fix-buffer-overflow-and-tainted-format-string.patch of Package alsa-utils (Revision cc23c84ac72f466950072609a4001bb1)

Places