Skip to content

Engine: Replace all potentially dangerous uses of strtok(NULL,) with Bstrtoken()

Dino Bollinger requested to merge dibollinger/eduke32:strtok_purge into master

Because this function stores state statically, using it in a loop can result in unexpected behavior when another function called during the loop invokes strtok() with a different buffer as argument.

I've seen this happen in practice when settings.cfg was being parsed:

 1.5275s INFO| Initializing console...
 1.6422s INPT| No game controllers found.
 1.6442s INFO| Executing settings.cfg
 1.6452s INFO| r_displayindex 1
 1.6460s  GFX| Detecting video modes for display 1 (LS32A600U)...
 1.6589s  GFX| Setting video mode 1920x1080 (32-bpp windowed).
 1.7316s  GFX| OpenGL driver: NVIDIA GeForce RTX 3060/PCIe/SSE2 4.6.0 NVIDIA 536.67
 1.7316s  GFX| OpenGL context: version 4.6
 1.7316s  GFX| Refresh rate: 75.00Hz.
 1.7698s INFO| Opened texturecache as cache file.
 1.7831s INFO| art: unknown command or cvar

Upon reaching r_displayindex 1, on the next iteration, strtok() somehow got a messed up state, that caused it to continue at a completely different location in memory, possibly where the extension for ART files was being stored.

The result was that it saw the string .art, and interpreted it as a OSD command, resulting in the above error message, despite this string not being present in either the file, or the buffer that was loaded in memory (Verified using gdb).

The concrete problem in the case of settings.cfg being that it then no longer parses the rest of the file as soon as this happens.

Edited by Dino Bollinger

Merge request reports