mixer: chan_play dereferences channeldata[-1] when Mix_PlayChannelTimed/Mix_FadeInChannelTimed returns -1

[coderabbitai]

Bug Report

In src_c/mixer.c, the chan_play function does not check whether channelnum is -1 after calling Mix_PlayChannelTimed or Mix_FadeInChannelTimed before indexing into channeldata. If either SDL mixer call fails and returns -1, the subsequent code dereferences channeldata[-1], which is out-of-bounds and can corrupt memory.

Affected code

src_c/mixer.c — chan_play function, around the block:

Py_XDECREF(channeldata[channelnum].sound);
Py_XDECREF(channeldata[channelnum].queue);
channeldata[channelnum].sound = Py_NewRef(sound);
channeldata[channelnum].queue = NULL;

A guard checking channelnum != -1 (raising an SDL error when -1) is missing here, similar to the guard already present in pgSound_Play.

References

• Identified during review of PR Replace Py_INCREF + assignment/return with Py_NewRef #3797 (comment: Replace Py_INCREF + assignment/return with Py_NewRef #3797 (comment))
• Reported by @oddbookworm

[oddbookworm]

Testing this led me to a larger issue if you change the number of channels after already allocating a channel. No sounds plays here, but it does run successfully without error:

import pygame

pygame.mixer.init()
print(f"num channels: {pygame.mixer.get_num_channels()}")
snd = pygame.mixer.Sound("examples/data/boom.wav")
chan = pygame.mixer.Channel(7)
pygame.mixer.set_num_channels(5)
chan.play(snd)
x = 0
while (x < 20_000_000):
    x += 1

I would expect an error to occur here when you try to play a channel that is no longer valid. But looking at set_num_channels in mixer.c, we only reallocate the array if the number of channels increases and that appears to do weird things with mixer