jamulussoftware / jamulussoftware/jamulus

Investigate clientChannels[] out of bounds read/crash

オープン
#3,926 コメント 2 件 リアクション 0 件 担当者 1 名 @ann0see が担当を希望しています GitHub で見る
AI bug good first issue
主要言語
C
スター
1.1k
フォーク
248
平均マージ
2日 3時間
マージ済み PR(30日)
9

説明

**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`).

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。