Skip to content

Commit

Permalink
aplay: fix buffer overflow and tainted format string
Browse files Browse the repository at this point in the history
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: #246
Signed-off-by: Mingjie Shen <[email protected]>
Signed-off-by: Jaroslav Kysela <[email protected]>
  • Loading branch information
szsam authored and perexg committed Dec 8, 2023
1 parent 004d085 commit 4ce6a0a
Showing 1 changed file with 10 additions and 10 deletions.
20 changes: 10 additions & 10 deletions aplay/aplay.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 4ce6a0a

Please sign in to comment.