Skip to content

Commit 8463da5

Browse files
committed
AudioModule: validate codec2 header and bound the RX decode reads
Two issues reachable from a crafted AUDIO_APP payload: The RX path built a temp codec2 from rx_encode_frame[3] whenever the frame header did not match ours. codec2_create returns NULL for an invalid mode byte, and the next call dereferenced it. Only decode frames that carry our own header (magic + mode) and drop the rest, so the untrusted mode byte never reaches codec2_create. The decode loop advanced by the frame size while testing only i < rx_encode_frame_index, so a payload length that was not a multiple of the frame size read past the received data and could read past rx_encode_frame. Bound each read to i + frameSize <= the received length clamped to the buffer, and clamp the receive memcpy to the buffer. Behavior change: audio frames whose codec2 mode differs from this node's configured mode are dropped instead of decoded with a temporary codec.
1 parent f18a8b0 commit 8463da5

1 file changed

Lines changed: 16 additions & 14 deletions

File tree

src/modules/esp32/AudioModule.cpp

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -68,23 +68,22 @@ void run_codec2(void *parameter)
6868
}
6969
if (audioModule->radio_state == RadioState::rx) {
7070
size_t bytesOut = 0;
71-
if (memcmp(audioModule->rx_encode_frame, &audioModule->tx_header, sizeof(audioModule->tx_header)) == 0) {
72-
for (int i = 4; i < audioModule->rx_encode_frame_index; i += audioModule->encode_codec_size) {
71+
// Reject frames not carrying our own header (magic + mode); an invalid mode byte
72+
// would else NULL-deref via codec2_create. The bound keeps reads inside the buffer.
73+
const int headerLen = sizeof(audioModule->tx_header);
74+
const int frameSize = audioModule->encode_codec_size;
75+
int limit = audioModule->rx_encode_frame_index;
76+
if (limit > (int)sizeof(audioModule->rx_encode_frame))
77+
limit = sizeof(audioModule->rx_encode_frame);
78+
if (frameSize > 0 && limit >= headerLen &&
79+
memcmp(audioModule->rx_encode_frame, &audioModule->tx_header, headerLen) == 0) {
80+
for (int i = headerLen; i + frameSize <= limit; i += frameSize) {
7381
codec2_decode(audioModule->codec2, audioModule->output_buffer, audioModule->rx_encode_frame + i);
7482
i2s_write(I2S_PORT, &audioModule->output_buffer, audioModule->adc_buffer_size, &bytesOut,
7583
pdMS_TO_TICKS(500));
7684
}
7785
} else {
78-
// if the buffer header does not match our own codec, make a temp decoding setup.
79-
CODEC2 *tmp_codec2 = codec2_create(audioModule->rx_encode_frame[3]);
80-
codec2_set_lpc_post_filter(tmp_codec2, 1, 0, 0.8, 0.2);
81-
int tmp_encode_codec_size = (codec2_bits_per_frame(tmp_codec2) + 7) / 8;
82-
int tmp_adc_buffer_size = codec2_samples_per_frame(tmp_codec2);
83-
for (int i = 4; i < audioModule->rx_encode_frame_index; i += tmp_encode_codec_size) {
84-
codec2_decode(tmp_codec2, audioModule->output_buffer, audioModule->rx_encode_frame + i);
85-
i2s_write(I2S_PORT, &audioModule->output_buffer, tmp_adc_buffer_size, &bytesOut, pdMS_TO_TICKS(500));
86-
}
87-
codec2_destroy(tmp_codec2);
86+
LOG_WARN("Audio: dropping frame with mismatched or short codec2 header");
8887
}
8988
}
9089
}
@@ -273,9 +272,12 @@ ProcessMessage AudioModule::handleReceived(const meshtastic_MeshPacket &mp)
273272
if ((moduleConfig.audio.codec2_enabled) && (myRegion->profile->audioPermitted)) {
274273
auto &p = mp.decoded;
275274
if (!isFromUs(&mp)) {
276-
memcpy(rx_encode_frame, p.payload.bytes, p.payload.size);
275+
size_t n = p.payload.size;
276+
if (n > sizeof(rx_encode_frame)) // bound a crafted payload to the RX buffer
277+
n = sizeof(rx_encode_frame);
278+
memcpy(rx_encode_frame, p.payload.bytes, n);
277279
radio_state = RadioState::rx;
278-
rx_encode_frame_index = p.payload.size;
280+
rx_encode_frame_index = n;
279281
// Notify run_codec2 task that the buffer is ready.
280282
BaseType_t xHigherPriorityTaskWoken = pdFALSE;
281283
vTaskNotifyGiveFromISR(codec2HandlerTask, &xHigherPriorityTaskWoken);

0 commit comments

Comments
 (0)