jamulussoftware / jamulussoftware/jamulus

Investigate clientChannels[] out of bounds read/crash

Abierto
#3,926 2 comentarios 0 reacciones 1 asignado Ver en GitHub

@ann0see ya está trabajando en esto.

Desde el 27/8/2026.

AI bug good first issue
Lenguaje dominante
C
Estrellas
1.1k
Forks
248
Merge medio
2 d 3 h
PR fusionados (30 d)
9

Descripción

**Describe the bug**

INVALID_INDEX might not be checked everywhere correctly. AI found out that there could be an out of bounds read possiblity in client.cpp if calling SetRemoteChanGain:

https://github.com/jamulussoftware/jamulus/blob/508f1f3d3ea41b0da76da128ee5d3f3f1b2fafc7/src/client.cpp#L1020-L1031

**To Reproduce**

Not tested. Would probably need some server side trigger with invalid Channel ID.
Probably worth checking on the protocol level for invalid IDs.

Ox Alpha (GLM 5.3-Flash) suggested:

> Guard the call site (minimal change):

```cpp
if ( iChanID != INVALID_INDEX && bMuteMeInPersonalMix ) { ... }
```

and/or reject invalid IDs in `EvaluateClientIDMes`; optionally add a defensive `Q_ASSERT`/range check in `SetRemoteChanGain`/`SetRemoteChanPan` mirroring `OnControllerInFaderLevel` (`client.cpp:956`). Related latent gap: `OnControllerInPanValue` (`client.cpp:965-975`) lacks the bounds check its fader sibling has.

**Expected behavior**

No crash

**AI analysis**

- `src/protocol.cpp:1046-1063` (`EvaluateClientIDMes`) — validates only body size (1 byte); the ID value itself (0–255) is passed through unchecked.
- `src/client.cpp:1010-1036` (`CClient::OnClientIDReceived`):

```cpp
int iChanID = FindClientChannel ( iServerChanID, true ); // should always return channel 0
...
if ( bMuteMeInPersonalMix )
{
SetRemoteChanGain ( iChanID, 0, false ); // iChanID can be INVALID_INDEX (-1)
}
```

- `src/client.cpp:1833-1885` (`FindClientChannel`) returns `INVALID_INDEX` (-1) when `iServerChannelID < 0 || >= MAX_NUM_CHANNELS` **or** when all 150 client channel slots are occupied.
- `src/client.cpp:506-539` (`SetRemoteChanGain`): `&clientChannels[iId]` with no bounds check →
- timer inactive: OOB write at `client.cpp:535` (`clientChan->oldGain = clientChan->newGain = fGain;`) plus OOB read of `iServerChannelID` fed to `Channel.SetRemoteChanGain()` (that callee is range-checked, `channel.cpp:302`);
- timer active: OOB write at `client.cpp:522`, and `minGainOrPanId = -1` causes `OnTimerRemoteChanGainOrPan` (`client.cpp:546-548`) to iterate from index −1 afterwards.
- `clientChannels` is a fixed `CClientChannel[150]` member array (`src/client.h:392`).

Contrast with the correct pattern used 700 lines earlier: `OnMuteStateHasChangedReceived` checks `if ( iChanID != INVALID_INDEX )` (`client.cpp:340-346`).

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.