Phase 0.5: S1 + S2 security hardening - #2
Merged
Conversation
The chromiumFlags storage previously accepted any free-form string in the `customSwitches` field and replayed every line through `app.commandLine.appendSwitch()` on next launch. A renderer-side compromise (XSS, malicious widget, supply chain) could therefore persist switches like `remote-debugging-port=9222`, `disable-web-security`, `proxy-server`, `js-flags=--allow-natives-syntax`, or `user-data-dir=<attacker-path>` that would weaken the security posture of the next launch. Fix: introduce CUSTOM_SWITCH_ALLOWLIST in storage/chromiumFlags.ts — a narrow set of GPU, rendering, scaling, and safe-diagnostic switches that have a legitimate troubleshooting use. Anything outside the list is dropped at normalisation time (both save and load paths) and the dropped names are logged via electron-log. Switches that affect network behaviour, debugger exposure, JS-engine tuning, extension loading, profile directories, or the same-origin policy are deliberately excluded. The parser is now exported from storage and used by overlayManager, so the application path and the validation path share a single source of truth for the syntax. Addresses finding S1 in docs/ARCHITECTURE_REVIEW.md. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Several entry points in the native N-API addon dereference pointers returned by the iRacing SDK without checking for NULL, and read JS-supplied indices and Napi::Value slots without bounds-checking. Any of these could be reached from JS by calling the wrapper before iRacing is running, by passing an out-of-range integer, or by passing a non-numeric argument — and would crash the Electron main process rather than failing cleanly. Fixes in this commit: * GetTelemetryData: irsdk_getHeader() returns NULL until shared memory is mapped. The previous code dereferenced header->numVars unguarded. Now returns an empty object when the SDK has no header. * GetTelemetryVarByIndex: irsdk_getVarHeaderEntry() returns NULL for indices outside [0, numVars) or when uninitialised; _data is NULL until the first telemetry read. Both are now checked. Also adds a bounds check on headerVar->type before indexing irsdk_VarTypeBytes. * GetTelemetryVar (JS entry): fixes a pre-existing uninitialised- variable bug where the common numeric-arg path left varIndex with stack garbage that flowed into the index-based getter. Now initialised to 0 and overwritten from info[0] only when valid. * GetTelemetryVar (string overload): rejects the -1 return from irsdk_varNameToIndex (unknown name / uninitialised SDK) instead of passing it to the index-based getter. * BroadcastMessage: previous guard rejected length <= 2 but then read info[3] out of bounds for length-3 calls, and never type-checked info[1] or info[2]. Now requires length >= 3 with all three numeric; info[3] is read defensively and the two camera-switch variants reject when it's missing. Preserves the existing accepted call set (the 3-arg ChatCommand/PitCommand/FFBCommand variants in irsdk-node.ts still work). Native code requires `npm run package` (or a forge rebuild) to take effect — there is no separate electron-rebuild script. Addresses finding S2 in docs/ARCHITECTURE_REVIEW.md. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
customSwitches(was free-form passthrough). Dropped names are logged.GetTelemetryData,GetTelemetryVarByIndex,GetTelemetryVar); tightenBroadcastMessagearg validation.Refs
docs/ARCHITECTURE_REVIEW.mdfindings S1 and S2.Test plan
npm run package/ forge rebuild)🤖 Generated with Claude Code