From a2a2d223d5b371bcfbeeb57769359751a273fc95 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 2 Jun 2026 22:47:15 -0700 Subject: [PATCH 1/2] fix(llf): harden degenerate/malformed inputs across particle lights Addresses the minor hardening items in #87 (pre-existing, surfaced by CodeRabbit on #56): - Particle.cpp: guard the dimmer interpolation against lightFadeEnd <= lightFadeStart so equal engine fade globals can't divide by zero (NaN/Inf into GPU lighting). - ParticleLights.cpp: std::sort the get_configs results before the 'first wins' duplicate dedupe so the surviving config is deterministic across installs (both ParticleLights and Gradients loops). - ParticleLights.cpp: require exactly 6 or 8 hex digits for gradient colors so malformed widths (#1, #12345) fail closed instead of packing a garbage 32-bit color. - Upscaling.cpp: clamp the stored streamlineLogLevel (not just the UI index) so a stale/hand-edited out-of-range value can't persist through the save / restart-diff paths. Closes #87 --- src/Features/LightLimitFix/Particle.cpp | 4 +++- src/Features/LightLimitFix/ParticleLights.cpp | 10 +++++++++- src/Features/Upscaling.cpp | 12 +++++++----- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/Features/LightLimitFix/Particle.cpp b/src/Features/LightLimitFix/Particle.cpp index b023ff8d21..7a3a61ef6e 100644 --- a/src/Features/LightLimitFix/Particle.cpp +++ b/src/Features/LightLimitFix/Particle.cpp @@ -526,7 +526,9 @@ void LightLimitFix::AddCachedParticleLights(eastl::vector& lightsData float distance = CalculateLightDistance(light.positionWS[0].data, light.radius); float dimmer = 0.0f; - if (distance < lightFadeStart || lightFadeEnd == 0.0f) + // lightFadeEnd <= lightFadeStart guards a zero/negative denominator below + // (equal engine globals would otherwise divide by zero → NaN into lighting). + if (distance < lightFadeStart || lightFadeEnd == 0.0f || lightFadeEnd <= lightFadeStart) dimmer = 1.0f; else if (distance <= lightFadeEnd) dimmer = 1.0f - ((distance - lightFadeStart) / (lightFadeEnd - lightFadeStart)); diff --git a/src/Features/LightLimitFix/ParticleLights.cpp b/src/Features/LightLimitFix/ParticleLights.cpp index df0cf38ac4..83da9cafef 100644 --- a/src/Features/LightLimitFix/ParticleLights.cpp +++ b/src/Features/LightLimitFix/ParticleLights.cpp @@ -31,6 +31,9 @@ void ParticleLights::GetConfigs() logger::info("[LLF] Loading particle lights configs"); auto configs = clib_util::distribution::get_configs("Data\\ParticleLights", "", ".ini"); + // get_configs order is filesystem-dependent; sort so the "first wins" + // duplicate policy below is deterministic across installs. + std::sort(configs.begin(), configs.end()); if (configs.empty()) { logger::warn("[LLF] No .ini files were found within the Data\\ParticleLights folder, aborting..."); @@ -78,6 +81,8 @@ void ParticleLights::GetConfigs() logger::info("[LLF] Loading particle lights gradients configs"); auto configs = clib_util::distribution::get_configs("Data\\ParticleLights\\Gradients", "", ".ini"); + // Deterministic "first wins" across installs (see above). + std::sort(configs.begin(), configs.end()); if (configs.empty()) { logger::warn("[LLF] No .ini files were found within the Data\\ParticleLights\\Gradients folder, aborting..."); @@ -112,7 +117,10 @@ void ParticleLights::GetConfigs() if (str.starts_with(prefix2)) str.remove_prefix(prefix2.size()); - bool matches = std::strspn(str.data(), cset.data()) == str.size(); + // Require exactly 6 (RRGGBB) or 8 (AARRGGBB) hex digits so malformed + // widths like "#1" / "#12345" fail closed instead of packing a garbage color. + bool matches = (str.size() == 6 || str.size() == 8) && + std::strspn(str.data(), cset.data()) == str.size(); if (matches) { try { diff --git a/src/Features/Upscaling.cpp b/src/Features/Upscaling.cpp index 7ca57363fb..52ad39f2c3 100644 --- a/src/Features/Upscaling.cpp +++ b/src/Features/Upscaling.cpp @@ -536,11 +536,13 @@ void Upscaling::DrawSettings() if (ImGui::TreeNodeEx("Backend Diagnostics")) { // Streamline log level selection const char* logLevels[] = { "Off", "Default", "Verbose" }; - // Clamp before use: streamlineLogLevel is JSON-persisted and could be out - // of range (or a value that overflows the int cast) on a stale or - // hand-edited config; an unclamped value would index logLevels OOB. - int logLevelIdx = std::clamp(static_cast(settings.streamlineLogLevel), - 0, IM_ARRAYSIZE(logLevels) - 1); + // Clamp the stored value (not just the index): streamlineLogLevel is + // JSON-persisted and could be out of range on a stale/hand-edited config. + // Sanitizing only the local index would let the bad value persist through + // the save / restart-diff paths even if the user never opens this combo. + settings.streamlineLogLevel = std::min(settings.streamlineLogLevel, + static_cast(IM_ARRAYSIZE(logLevels) - 1)); + int logLevelIdx = static_cast(settings.streamlineLogLevel); if (ImGui::Combo("Streamline Logging", &logLevelIdx, logLevels, IM_ARRAYSIZE(logLevels))) { settings.streamlineLogLevel = static_cast(logLevelIdx); } From 70348b20687bf767fd291d0bf7b2267e6e7af1fc Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Wed, 3 Jun 2026 02:58:52 -0700 Subject: [PATCH 2/2] fix: address review comments on input hardening - ParticleLights: validate hex digits with bounded find_first_not_of instead of strspn on string_view::data(), which is not guaranteed to be NUL-terminated. - Upscaling: clamp streamlineLogLevel in LoadSettings so a stale or out-of-range JSON value is sanitized on every load, not only when the Backend Diagnostics tree node is expanded. Drop the now-redundant in-tree clamp whose comment claimed coverage it did not provide. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Features/LightLimitFix/ParticleLights.cpp | 4 +++- src/Features/Upscaling.cpp | 12 ++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Features/LightLimitFix/ParticleLights.cpp b/src/Features/LightLimitFix/ParticleLights.cpp index 83da9cafef..938bc0220e 100644 --- a/src/Features/LightLimitFix/ParticleLights.cpp +++ b/src/Features/LightLimitFix/ParticleLights.cpp @@ -119,8 +119,10 @@ void ParticleLights::GetConfigs() // Require exactly 6 (RRGGBB) or 8 (AARRGGBB) hex digits so malformed // widths like "#1" / "#12345" fail closed instead of packing a garbage color. + // find_first_not_of is bounded to the view; strspn would rely on + // str.data() being NUL-terminated, which string_view doesn't guarantee. bool matches = (str.size() == 6 || str.size() == 8) && - std::strspn(str.data(), cset.data()) == str.size(); + str.find_first_not_of(cset) == std::string_view::npos; if (matches) { try { diff --git a/src/Features/Upscaling.cpp b/src/Features/Upscaling.cpp index 52ad39f2c3..6cdaf71bd4 100644 --- a/src/Features/Upscaling.cpp +++ b/src/Features/Upscaling.cpp @@ -536,12 +536,8 @@ void Upscaling::DrawSettings() if (ImGui::TreeNodeEx("Backend Diagnostics")) { // Streamline log level selection const char* logLevels[] = { "Off", "Default", "Verbose" }; - // Clamp the stored value (not just the index): streamlineLogLevel is - // JSON-persisted and could be out of range on a stale/hand-edited config. - // Sanitizing only the local index would let the bad value persist through - // the save / restart-diff paths even if the user never opens this combo. - settings.streamlineLogLevel = std::min(settings.streamlineLogLevel, - static_cast(IM_ARRAYSIZE(logLevels) - 1)); + // streamlineLogLevel is sanitized in LoadSettings (runs on every load, + // not gated on this node being expanded), so the stored value is in range. int logLevelIdx = static_cast(settings.streamlineLogLevel); if (ImGui::Combo("Streamline Logging", &logLevelIdx, logLevels, IM_ARRAYSIZE(logLevels))) { settings.streamlineLogLevel = static_cast(logLevelIdx); @@ -704,6 +700,10 @@ void Upscaling::LoadSettings(json& o_json) logger::warn("[Upscaling] Loaded presetDLSS {} out of range, resetting to 0 (Default)", settings.presetDLSS); settings.presetDLSS = 0; } + if (settings.streamlineLogLevel > 2) { // Off, Default, Verbose + logger::warn("[Upscaling] Loaded streamlineLogLevel {} out of range, clamping to 2", settings.streamlineLogLevel); + settings.streamlineLogLevel = 2; + } // Re-apply FoveatedRender's cross-feature clamp now that the JSON // re-assign above has overwritten anything it set during its own // LoadSettings (which fired before this block ran). Idempotent — no-op