Skip to content

Buffer overflow in audio player. #132

Description

@Arslan8

I verified that the current waveplayer.c still allocates str as FILEMGR_FILE_NAME_SIZE + 20 and then formats the filename into it using unbounded sprintf(). ([GitHub]1) With a 40-character filename, even "Playing file (1/1): " + filename + NUL requires 61 bytes, already exceeding a 60-byte buffer. The missing termination from the earlier strncpy() can make the condition worse.

Buffer overflow in GetFileInfo() when displaying long WAV filenames

Describe the set-up

  • Board: STM32F769I-DISCO / STM32F769I-Discovery
  • Example: Projects/STM32F769I-Discovery/Applications/Audio/Audio_playback_and_record
  • Affected revision: c2ecfd2d
  • Affected file: Src/waveplayer.c
  • Affected function: GetFileInfo()

The issue is independent of a specific compiler and is caused by an unchecked formatted write to a fixed-size stack buffer.

Describe the bug

GetFileInfo() uses sprintf() to construct a display string containing the selected WAV filename:

uint8_t str[FILEMGR_FILE_NAME_SIZE + 20];

sprintf((char *)str, "Playing file (%d/%d): %s",
        file_idx + 1, FileList.ptr,
        (char *)FileList.file[file_idx].name);

FILEMGR_FILE_NAME_SIZE is 40, so str is 60 bytes.

However, the fixed text alone requires 20 bytes even for single-digit indices:

Playing file (1/1):

A 40-character filename therefore requires at least:

20-byte prefix + 40-byte filename + 1-byte NUL = 61 bytes

This exceeds the 60-byte destination buffer. Larger values of file_idx or FileList.ptr increase the required size further.

In addition, during file enumeration the filename is copied using:

strncpy((char *)FileList.file[FileList.ptr].name,
        (char *)fn, FILEMGR_FILE_NAME_SIZE);

If the source filename is at least FILEMGR_FILE_NAME_SIZE bytes long, strncpy() does not append a terminating NUL byte. The later %s conversion in sprintf() can therefore also read beyond the filename buffer until an unrelated NUL byte is encountered.

As a result, a sufficiently long filename on the removable media can cause an out-of-bounds read followed by a stack buffer overflow in GetFileInfo().

How To Reproduce

  1. Build and run the STM32F769I-Discovery Audio_playback_and_record application normally.

  2. Place a valid WAV file on the removable storage with a filename close to the maximum length accepted by the file manager, for example a 40-character filename.

  3. Insert the storage and allow the application to enumerate the WAV files.

  4. Select the long-named WAV file for playback.

  5. GetFileInfo() executes:

sprintf((char *)str, "Playing file (%d/%d): %s",
        file_idx + 1, FileList.ptr,
        (char *)FileList.file[file_idx].name);

The resulting string exceeds the size of str, causing memory beyond the stack buffer to be overwritten. If the filename was copied without a terminating NUL, %s may additionally read beyond the filename field.

Depending on the surrounding stack layout, the issue may result in memory corruption or a crash.

Additional context

The issue can be addressed by both ensuring that stored filenames are always NUL-terminated and using a bounded formatting function.

For example, the display operation can be changed to:

snprintf((char *)str, sizeof(str),
         "Playing file (%d/%d): %s",
         file_idx + 1, FileList.ptr,
         (char *)FileList.file[file_idx].name);

The filename copy should also reserve space for and explicitly add a terminator, for example:

strncpy((char *)FileList.file[FileList.ptr].name,
        (char *)fn,
        FILEMGR_FILE_NAME_SIZE - 1);

FileList.file[FileList.ptr].name[FILEMGR_FILE_NAME_SIZE - 1] = '\0';

Using both changes prevents the unterminated-source read and bounds the formatted write into str.

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingprojectsProjects-related (demos, applications, examples) issue or pull-request.

Type

Projects

Status
To do

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions