Skip to content

Codebase Audit #549

@x87

Description

@x87

CLEO5 Bug Audit

Core Runtime

Ordered roughly by severity.


1. Unsupported game versions crash during startup

  • WHERE: source/dllmain.cpp Starter::Starter, source/CleoBase.cpp CCleoInstance::Start() / OnScmInit2(), source/CGameVersionManager.cpp, source/CCodeInjector.h
  • WHAT: Unsupported or partially supported game versions can still crash during startup.
  • WHY: Starter only warns when the detected version is not GV_US10, but still calls CleoInstance.Start(). The address table contains many memory_und entries for non-US variants, and memory_und is defined as nullptr, which then flows into ReplaceFunction() and MemoryRead().

2. Child scripts leave stale pointers in parent

  • WHERE: source/CCustomScript.cpp CCustomScript::CCustomScript(...), source/CScriptEngine.cpp CScriptEngine::RemoveCustomScript()
  • WHAT: Label-spawned child scripts leave stale pointers in the parent and can later trigger use-after-free style behavior during teardown.
  • WHY: Child creation pushes this into parent->m_childScripts, but child destruction never removes itself from that vector. When the parent is later removed, it iterates m_childScripts and calls RemoveScript(childThread) on pointers that may already be invalid.

3. Format-string buffer overflow in ReadFormattedString

  • WHERE: source/CCustomOpcodeSystem.cpp ReadFormattedString()
  • WHAT: Script-controlled format strings can overflow a fixed stack buffer.
  • WHY: The parser builds fmtbufa[64] by copying flags, width, precision, *-expanded numeric values, and size modifiers with unchecked *fmta++ writes before calling sprintf_s.

4. Pointer-based string I/O trusts raw addresses

  • WHERE: source/CScriptEngine.cpp GetScriptStringParam(), source/CCustomOpcodeSystem.cpp GetStringParamWriteBuffer() and WriteStringParam(...)
  • WHAT: Pointer-based string input and output can read from or write to invalid or undersized memory.
  • WHY: The code only rejects pointers <= MinValidAddress, then uses strlen, memcpy, and effectively unbounded target sizes (0x7FFFFFFF) for pointer-backed buffers. Non-null but unmapped, unterminated, or too-small buffers can crash or corrupt memory.

5. Unchecked module export offsets

  • WHERE: source/CModuleSystem.cpp CModule::ModuleExport::LoadFromFile() / CModule::GetExport(), source/CCustomOpcodeSystem.cpp opcode_0AB1
  • WHAT: Malformed module metadata can redirect execution outside the loaded module buffer.
  • WHY: Export offsets are read from the module file and returned as ScriptDataRef without validating that the offset stays within data / size. opcode_0AB1 then swaps in the module base and jumps to that unchecked offset.

6. Wrong variable in path-boundary check

  • WHERE: source/CScriptEngine.cpp CScriptEngine::FindScriptByFilename()
  • WHAT: The path-boundary check uses the wrong string and can read out of bounds.
  • WHY: After suffix-matching against scriptPath, the code checks path[startPos - 1] != '\\' instead of checking scriptPath[startPos - 1]. If scriptPath is longer than the query, startPos - 1 can exceed the caller string length and also produce false negatives/positives.

Plugins


7. scan_file output-array overflow and unconditional success

  • WHERE: cleo_plugins/FileSystemOperations/FileSystemOperations.cpp opcode_0ADA, cleo_plugins/FileSystemOperations/FileUtils.cpp File::scan()
  • WHAT: scan_file can overflow its output-slot array and also reports success even when the scan did not fully match.
  • WHY: opcode_0ADA stores output pointers into SCRIPT_VAR* outputParams[35] with no cap, while the legacy scanner additionally writes outputParams[paramCount] = &charRead. The opcode then sets OPCODE_CONDITION_RESULT(true) unconditionally instead of checking conversion count.

8. Legacy scan_file infinite loop at EOF

  • WHERE: cleo_plugins/FileSystemOperations/FileUtils.cpp File::scan()
  • WHAT: The legacy scan_file path can loop forever at EOF.
  • WHY: The loop keeps appending readChar(handle) to readText, only breaks on stream error, and only checks EOF inside the read == paramCount branch. A partial/non-matching scan at EOF never reaches a terminating condition.

9. scan_string buffer overflow on %s conversions

  • WHERE: cleo_plugins/Text/Text.cpp opcode_0AD4
  • WHAT: scan_string can overflow caller memory on %s conversions.
  • WHY: It forwards the script-supplied format string directly to sscanf and passes writable pointers without width limits. That can overflow raw pointer targets, legacy string variables, and even the strict-mode temporary string buffers.

10. Stale static arguments in CallFunctionGeneric

  • WHERE: cleo_plugins/MemoryOperations/MemoryOperations.cpp CallFunctionGeneric()
  • WHAT: Missing arguments in compatibility mode can reuse stale values from a previous call.
  • WHY: The function uses a static arguments[32] array and only overwrites the first min(numArg, inputArgCount) entries. If a script declares more args than it actually provides in legacy mode, the remaining pushed values come from old calls instead of zeroes.

11. Null/invalid model dereference in get_name_of_vehicle_model

  • WHERE: cleo_plugins/Text/Text.cpp opcode_0ADB
  • WHAT: Invalid model ids can produce out-of-bounds access or null dereference.
  • WHY: modelIndex is used directly with GetModelInfo / ms_modelInfoPtrs, and model->m_szGameName is dereferenced without verifying that the model pointer is non-null and actually points to a vehicle model.

12. Dangling pointers in breakpoint bookkeeping

  • WHERE: cleo_plugins/DebugUtils/DebugUtils.cpp Opcode_Breakpoint, OnDrawingFinished, OnScriptProcessBefore
  • WHAT: Breakpoint bookkeeping can retain dangling CRunningScript* pointers.
  • WHY: Paused scripts are stored in pausedScripts as raw pointers, but the plugin only clears that list on global finalization or manual resume. If a paused script is terminated elsewhere, later UI or scheduler paths dereference stale pointers.

13. Stale recursive-search state after script deletion

  • WHERE: cleo_plugins/GameEntities/GameEntities.cpp charSearchState, carSearchState, objectSearchState, opcodes 0AE1 / 0AE2 / 0AE3
  • WHAT: Recursive search state survives per-script deletion and can leak stale iteration state across script lifetimes.
  • WHY: The maps are keyed by raw CRunningScript* and are only cleared in OnScriptsFinalize(). Individual script deletion leaves stale entries behind, so pointer reuse can make a later script resume an old search index.

14. Sphere search ignores radius >= 1000

  • WHERE: cleo_plugins/GameEntities/GameEntities.cpp opcodes 0AE1, 0AE2, 0AE3
  • WHAT: The "in sphere" search stops honoring the sphere once radius >= 1000.0f.
  • WHY: All three handlers only apply the distance test inside if (radius < 1000.0f). For larger radii they return the first matching pooled entity anywhere in the world.

15. Undefined behavior in shift and bit opcodes

  • WHERE: cleo_plugins/Math/Math.cpp opcodes 0B15, 0B16, 0B1C, 0B1D, 0B1E, 2701, 2702, 2703
  • WHAT: Several shift and bit opcodes invoke undefined behavior on edge inputs.
  • WHY: Shift counts come straight from script input with no range checks, Sign_Extend can evaluate 1 << 31, signed left shifts can overflow into the sign bit, and 2701-2703 accept bit index 31 while using the signed literal 1 << bitIndex.

16. Silent truncation of INI numeric values

  • WHERE: cleo_plugins/IniFiles/IniFiles.cpp Script_InifileGetInt and Script_InifileGetFloat
  • WHAT: Long numeric INI values are silently truncated and then parsed as successful results.
  • WHY: Both readers use char buff[32] with GetPrivateProfileString(). Windows truncates into the small buffer, and the code then feeds the truncated prefix into strtol / strtof and treats partial parses as success.

17. Incorrect SendInput flags for key emulation

  • WHERE: cleo_plugins/Input/main.cpp Input::SendKeyEvent, opcodes 2083 and 2084
  • WHAT: Synthetic keyboard events are built with incorrect SendInput flags.
  • WHY: The code fills wScan for most keys but never sets KEYEVENTF_SCANCODE, and it adds KEYEVENTF_EXTENDEDKEY broadly instead of only for true extended keys. That means many emulated keys depend on misused API semantics.

18. Audio length/progress can return invalid values

  • WHERE: cleo_plugins/Audio/CAudioStream.cpp GetLength() and GetProgress(), reached by opcodes 0AAF, 2501, 2507, 250B
  • WHAT: Length and progress queries can return invalid values, including negative values or NaN.
  • WHY: The code handles BASS_ChannelGetPosition(...) == -1 but does not validate BASS_ChannelGetLength(...). For streams where length is unavailable, it still converts the invalid result to seconds and divides by it.

19. 3D audio muted at world origin

  • WHERE: cleo_plugins/Audio/C3DAudioStream.cpp UpdatePosition() and CalculateVolume()
  • WHAT: 3D streams use {0,0,0} as an "uninitialized previous position" sentinel, which breaks valid audio sources at the world origin and delays audibility after first placement.
  • WHY: placed is only set when prevPos.Magnitude() > 0.0f. A real source at the world origin never becomes "placed", and newly positioned sources are treated as unplaced for the first update because prevPos starts at zero.

Test Blind Spots

These are not separate code bugs; they are gaps that currently fail to protect the bugs above.


T1. No coverage for scan_file (opcode 0ADA)

  • WHERE: tests/cleo_tests/FilesystemOperations and cleo_plugins/FileSystemOperations/FileSystemOperations.cpp opcode_0ADA
  • WHAT: There is no automated coverage for scan_file.
  • WHY: The repository has no 0ADA.txt, so the current suite never exercises the opcode that currently has an uncapped output array and unconditional success condition.

T2. ReadFormattedString not stress-tested for overflow

  • WHERE: tests/cleo_tests/SDK/ReadParamsFormatted.txt and source/CCustomOpcodeSystem.cpp ReadFormattedString()
  • WHAT: The SDK tests cover malformed specifiers and small output buffers, but not oversized width/precision expansions.
  • WHY: Nothing in the test file stresses long flag sequences or *-expanded width/precision values that would overrun fmtbufa[64].

T3. Module-import coverage is happy-path only

  • WHERE: tests/cleo_tests/Cleo/0AB1.txt and source/CModuleSystem.cpp / source/CCustomOpcodeSystem.cpp
  • WHAT: Module-import coverage is happy-path only.
  • WHY: The test only imports one valid export from one valid module and checks Fibonacci output. It does not cover malformed export offsets, nested imports, or state-restoration failures after module calls.

T4. Key-emulation tests do not isolate press vs release

  • WHERE: tests/cleo_tests/Input/2083.txt, tests/cleo_tests/Input/2084.txt, cleo_plugins/Input/main.cpp
  • WHAT: The key-emulation tests do not validate press and release independently.
  • WHY: Both tests use the same StrokeKey() helper that always performs press plus release. One opcode could be broken and both tests could still pass because the pair still types the cheat code.

T5. get_name_of_vehicle_model only tested on happy path

  • WHERE: tests/cleo_tests/Text/0ADB.txt and cleo_plugins/Text/Text.cpp opcode_0ADB
  • WHAT: get_name_of_vehicle_model is only tested on the happy path.
  • WHY: The test only checks model 400 and static-buffer storage. There is no invalid-model, null-model, or wrong-type case to expose the unchecked dereference.

T6. scan_string misses dangerous %s overflow cases

  • WHERE: tests/cleo_tests/Text/0AD4.txt and cleo_plugins/Text/Text.cpp opcode_0AD4
  • WHAT: scan_string coverage misses the dangerous %s overflow cases and barely checks special-token behavior.
  • WHY: The test suite exercises normal strings and %*, but it does not assert the opcode condition for the %* case, does not cover %%, and does not feed oversized tokens that would overflow raw destinations.

T7. Sphere-search tests miss boundary and large-radius cases

  • WHERE: tests/cleo_tests/GameEntities/0AE1.txt, 0AE2.txt, 0AE3.txt and cleo_plugins/GameEntities/GameEntities.cpp
  • WHAT: Sphere-search tests miss the boundary and large-radius cases.
  • WHY: The current tests only prove that nearby entities are found. They do not place entities just outside the radius, exactly on the boundary, or test radius >= 1000.0f, so the "radius ignored" bug goes undetected.

T8. Audio progress/length coverage is happy-path only

  • WHERE: tests/cleo_tests/Audio/0AAF.txt, 2501.txt, 2507.txt, 250B.txt and cleo_plugins/Audio/CAudioStream.cpp
  • WHAT: Audio progress/length coverage is happy-path only.
  • WHY: 0AAF.txt only checks a very short stream that truncates to 0 seconds, and the suite does not cover unavailable-length streams, paused/zero-speed cases, or divide-by-zero style progress math.

T9. INI read tests miss truncation and modern-mode regression

  • WHERE: tests/cleo_tests/IniFiles/0AF1.txt, 0AF3.txt, 0AF5.txt and cleo_plugins/IniFiles/IniFiles.cpp
  • WHAT: INI write/read coverage misses modern-mode regression cases and truncation edges.
  • WHY: The tests cover the legacy 0 deletion shortcut in CS4 mode, but there is no CS5 regression check and no long numeric-value case to expose silent truncation in the readers.

T10. 3D audio attachment coverage is incomplete

  • WHERE: tests/cleo_tests/Audio and cleo_plugins/Audio/Audio.cpp / cleo_plugins/Audio/C3DAudioStream.cpp
  • WHAT: 3D audio attachment coverage is incomplete.
  • WHY: The suite has 0AC2.txt and 0AC4.txt, but no 0AC3.txt or 0AC5.txt, so object/vehicle attachment paths are currently unguarded by automated tests.

Metadata

Metadata

Assignees

No one assigned

    Labels

    discussionFurther discussion is requested

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions