[Bug] Conditional jump on uninitialized variables in tunes.cpp

[JasonM22345]

Describe the bug

tunes.cpp - Conditional jump on uninitialized variables in Tunes::get_next_note(unsigned&, unsigned&, unsigned&, uint8_t&)

In the four-parameter overload of Tunes::get_next_note, when the internal 3-parameter version returns a non-Continue status, the out-parameters frequency and duration remain uninitialized. Immediately afterward, the code does

if (frequency == 0 || duration == 0) { … }

thus performing a conditional jump on uninitialized stack memory.

To Reproduce

Note: simply replaying the same payload does not reliably trigger the issue due to nondeterministic stack contents.

1. Launch the PX4 SITL executable under any runtime monitor or debugger.
2. Send this PLAY_TUNE MAVLink payload (hex):

fd290000b006b9020100c81fe9d86e816c60aa88e3b4d8c70599e6ece151e3790a35e91c49fbdf9e7904fec1c2d1a3706096a4db79

3. You will see that tunes.cpp’s four-parameter get_next_note() can perform a conditional on uninitialized variables.

Expected behavior

when the bug occurs, the three-parameter get_next_note(frequency, duration, silence) call returns a non-Continue status without ever writing to frequency or duration, and then the very next if (frequency == 0 || duration == 0) reads those uninitialized values, causing an unpredictable branch that may leave volume set to garbage or zero before the function returns Stop. As a result, even though the tune should simply end, you can get a stray or incorrect tune message (or volume) at the end due to that undefined read.

Screenshot / Media

No response

Flight Log

No response

Software Version

pxh> ver all
HW arch: PX4_SITL
PX4 git-hash: 5509061
PX4 version: 1.16.0 80 (17825920)
PX4 git-branch: main
OS: Linux
OS version: Release 5.15.133 (84903423)
Build datetime: Apr 2 2025 14:55:21
Build uri: localhost
Build variant: default
Toolchain: GNU GCC, 9.3.0
PX4GUID: 100655d4d9534954414c44494e4f303030
UNKNOWN MCU

Flight controller

SITL

Vehicle type

None

How are the different components wired up (including port information)

No response

Additional context

Solution (Add an early check for ret != Continue that zeroes all four output parameters and returns immediately, preventing any uninitialized reads in the subsequent volume check.):

File: https://github.com/PX4/PX4-Autopilot/blob/main/src/lib/tunes/tunes.cpp

Tunes::Status Tunes::get_next_note(unsigned &frequency, unsigned &duration, unsigned &silence, uint8_t &volume)
{
    if (_tunes_disabled) {
        frequency = 0;
        duration  = 0;
        silence   = 0;
        volume    = 0;
        return Tunes::Status::Stop;
    }

    Tunes::Status ret = get_next_note(frequency, duration, silence);

    // -- fix: if there are no more notes, zero out all outputs before touching frequency/duration
    if (ret != Tunes::Status::Continue) {    // -- fix
        frequency = 0;                       // -- fix
        duration  = 0;                       // -- fix
        silence   = 0;                       // -- fix
        volume    = 0;                       // -- fix
        return ret;                          // -- fix
    }

    // Check if note should not be heard -> adjust volume to 0 to be safe.
    if (frequency == 0 || duration == 0) {
        volume = 0;
    } else {
        volume = _volume;
    }

    return ret;
}

Original code:

Tunes::Status Tunes::get_next_note(unsigned &frequency, unsigned &duration, unsigned &silence, uint8_t &volume)
{
	if (_tunes_disabled) {
		frequency = 0;
		duration = 0;
		silence = 0;
		volume = 0;
		return Tunes::Status::Stop;
	}

	Tunes::Status ret = get_next_note(frequency, duration, silence);

	// Check if note should not be heard -> adjust volume to 0 to be safe.
	if (frequency == 0 || duration == 0) {
		volume = 0;

	} else {
		volume = _volume;
	}

	return ret;
}