mirror of
https://github.com/nimbold/Firelink.git
synced 2026-09-01 13:38:01 +00:00
fix(settings): harden persisted consumer state
- validate complete settings snapshots before replacing durable state - sanitize malformed proxy, user-agent, sidebar, speed, and path values at native and renderer hydration boundaries - add hostile persisted-settings regression coverage
This commit is contained in:
@@ -10912,8 +10912,12 @@ fn db_save_settings(
|
||||
crate::db::preserve_legacy_pairing_token(existing.as_deref(), &sanitized)?
|
||||
};
|
||||
let merged = crate::settings::canonicalize_torrent_network_settings(&merged)?;
|
||||
let decoded = crate::settings::decode_stored_settings(&serde_json::Value::String(merged.clone()))?;
|
||||
// Validate the complete settings document before replacing the durable
|
||||
// copy. A malformed renderer snapshot must not poison restart hydration or
|
||||
// leave the previous valid settings unrecoverable after this command
|
||||
// reports an error.
|
||||
crate::db::save_settings(&connection, &merged)?;
|
||||
let decoded = crate::settings::decode_stored_settings(&serde_json::Value::String(merged))?;
|
||||
let prevent_system_sleep = decoded.prevents_sleep_while_downloading;
|
||||
let prevent_display_sleep = decoded.prevents_display_sleep_while_downloading;
|
||||
if let Ok(mut cached) = app_state.scheduler_settings.write() {
|
||||
|
||||
@@ -229,6 +229,19 @@ pub fn canonicalize_torrent_network_settings(stored: &str) -> Result<String, Str
|
||||
"IPv6 Torrent bind address requires IPv6 transport to remain enabled".to_string(),
|
||||
);
|
||||
}
|
||||
// Renderer snapshots are also a persistence boundary. Remove malformed
|
||||
// scalar values before the document is written so a recoverable default
|
||||
// is not hidden behind a hostile value that will fail on the next save or
|
||||
// restart. Keep the network canonicalization above first so invalid text
|
||||
// fields retain their established empty-string representation.
|
||||
let state_value = if document.get("state").is_some() {
|
||||
document
|
||||
.get_mut("state")
|
||||
.ok_or_else(|| "persisted settings state is missing".to_string())?
|
||||
} else {
|
||||
&mut document
|
||||
};
|
||||
sanitize_persisted_setting_values(state_value);
|
||||
serde_json::to_string(&document)
|
||||
.map_err(|error| format!("failed to encode canonical settings: {error}"))
|
||||
}
|
||||
@@ -441,6 +454,11 @@ fn sanitize_persisted_setting_values(state: &mut Value) {
|
||||
sanitize_integer_setting(state, "maxConcurrentDownloads", |value| value.as_u64().is_some());
|
||||
sanitize_integer_setting(state, "perServerConnections", |value| value.as_i64().is_some());
|
||||
sanitize_integer_setting(state, "maxAutomaticRetries", |value| value.as_i64().is_some());
|
||||
sanitize_integer_setting(state, "proxyPort", |value| {
|
||||
value
|
||||
.as_u64()
|
||||
.is_some_and(|value| (1..=u16::MAX as u64).contains(&value))
|
||||
});
|
||||
sanitize_integer_setting(state, "torrentMaxOpenFiles", |value| {
|
||||
value
|
||||
.as_u64()
|
||||
@@ -465,6 +483,7 @@ fn sanitize_persisted_setting_values(state: &mut Value) {
|
||||
})
|
||||
});
|
||||
for key in [
|
||||
"isSidebarVisible",
|
||||
"torrentEnableDht",
|
||||
"torrentEnableDht6",
|
||||
"torrentEnablePex",
|
||||
@@ -474,6 +493,9 @@ fn sanitize_persisted_setting_values(state: &mut Value) {
|
||||
] {
|
||||
sanitize_boolean_setting(state, key);
|
||||
}
|
||||
for key in ["proxyHost", "customUserAgent"] {
|
||||
sanitize_string_setting(state, key);
|
||||
}
|
||||
sanitize_torrent_network_string(state, "torrentListenPort", |value| {
|
||||
crate::queue::normalize_torrent_port_spec(Some(value), "TCP listen ports").is_ok()
|
||||
});
|
||||
@@ -598,6 +620,12 @@ fn sanitize_boolean_setting(state: &mut serde_json::Map<String, Value>, key: &st
|
||||
}
|
||||
}
|
||||
|
||||
fn sanitize_string_setting(state: &mut serde_json::Map<String, Value>, key: &str) {
|
||||
if state.get(key).is_some_and(|value| !value.is_string()) {
|
||||
state.remove(key);
|
||||
}
|
||||
}
|
||||
|
||||
fn sanitize_torrent_network_string(
|
||||
state: &mut serde_json::Map<String, Value>,
|
||||
key: &str,
|
||||
@@ -1347,6 +1375,27 @@ mod tests {
|
||||
assert_eq!(settings.site_logins[0].id, "valid");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn malformed_proxy_and_user_agent_values_fall_back_to_safe_defaults() {
|
||||
let stored = json!({
|
||||
"state": {
|
||||
"proxyMode": "custom",
|
||||
"proxyHost": 123,
|
||||
"proxyPort": 70000,
|
||||
"customUserAgent": ["not-a-string"],
|
||||
"isSidebarVisible": "yes"
|
||||
}
|
||||
});
|
||||
|
||||
let settings = decode_stored_settings(&Value::String(stored.to_string())).unwrap();
|
||||
|
||||
assert!(matches!(settings.proxy_mode, crate::ipc::ProxyMode::Custom));
|
||||
assert!(settings.proxy_host.is_empty());
|
||||
assert_eq!(settings.proxy_port, 8080);
|
||||
assert!(settings.custom_user_agent.is_empty());
|
||||
assert!(settings.is_sidebar_visible);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_valid_torrent_network_settings() {
|
||||
let stored = json!({
|
||||
@@ -1386,7 +1435,10 @@ mod tests {
|
||||
"torrentPeerAgent": " Firelink/1.3.1 ",
|
||||
"torrentDhtMessageTimeout": 601,
|
||||
"torrentMaxConcurrentSeeds": 65,
|
||||
"torrentSeparateSeedSlots": "yes"
|
||||
"torrentSeparateSeedSlots": "yes",
|
||||
"proxyPort": 70000,
|
||||
"proxyHost": 123,
|
||||
"customUserAgent": ["not-a-string"]
|
||||
},
|
||||
"version": 6
|
||||
});
|
||||
@@ -1406,6 +1458,9 @@ mod tests {
|
||||
crate::queue::DEFAULT_TORRENT_MAX_CONCURRENT_SEEDS
|
||||
);
|
||||
assert_eq!(canonical["state"]["torrentSeparateSeedSlots"], false);
|
||||
assert!(canonical["state"].get("proxyPort").is_none());
|
||||
assert!(canonical["state"].get("proxyHost").is_none());
|
||||
assert!(canonical["state"].get("customUserAgent").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -65,6 +65,32 @@ describe('durable main-window and sidebar preferences', () => {
|
||||
.toEqual({ width: 1440, height: 900 });
|
||||
});
|
||||
|
||||
it('rejects malformed consumer values during hydration', () => {
|
||||
const merge = useSettingsStore.persist.getOptions().merge;
|
||||
expect(merge).toBeTypeOf('function');
|
||||
const current = useSettingsStore.getState();
|
||||
|
||||
expect(merge?.({
|
||||
proxyMode: 'custom',
|
||||
proxyHost: 123,
|
||||
proxyPort: 70000,
|
||||
customUserAgent: ['not-a-string'],
|
||||
isSidebarVisible: 'yes',
|
||||
lastCustomSpeedLimitKiB: Number.POSITIVE_INFINITY,
|
||||
approvedDownloadRoots: ['/safe', 42],
|
||||
speedLimitPresetValues: [1, '5', Number.NaN]
|
||||
}, current)).toMatchObject({
|
||||
proxyMode: 'custom',
|
||||
proxyHost: current.proxyHost,
|
||||
proxyPort: current.proxyPort,
|
||||
customUserAgent: current.customUserAgent,
|
||||
isSidebarVisible: current.isSidebarVisible,
|
||||
lastCustomSpeedLimitKiB: current.lastCustomSpeedLimitKiB,
|
||||
approvedDownloadRoots: ['/safe'],
|
||||
speedLimitPresetValues: [1]
|
||||
});
|
||||
});
|
||||
|
||||
it('uses the legacy localStorage value only when durable state is absent', () => {
|
||||
const originalWindow = globalThis.window;
|
||||
Object.defineProperty(globalThis, 'window', {
|
||||
|
||||
@@ -162,6 +162,21 @@ const sanitizeSiteLogins = (value: unknown): SiteLogin[] => {
|
||||
const persistedBoolean = (value: unknown, fallback: boolean) =>
|
||||
typeof value === 'boolean' ? value : fallback;
|
||||
|
||||
const persistedString = (value: unknown, fallback: string): string =>
|
||||
typeof value === 'string' ? value : fallback;
|
||||
|
||||
const persistedFiniteInteger = (
|
||||
value: unknown,
|
||||
minimum: number,
|
||||
maximum: number,
|
||||
fallback: number
|
||||
): number => {
|
||||
if (typeof value !== 'number' || !Number.isFinite(value) || !Number.isInteger(value)) {
|
||||
return fallback;
|
||||
}
|
||||
return value >= minimum && value <= maximum ? value : fallback;
|
||||
};
|
||||
|
||||
const tauriStorage: StateStorage = {
|
||||
getItem: async (name: string): Promise<string | null> => {
|
||||
if (name === 'firelink-settings') {
|
||||
@@ -928,6 +943,10 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
persisted.isFoldersCollapsed,
|
||||
foldersCollapsedFallback
|
||||
),
|
||||
isSidebarVisible: persistedBoolean(
|
||||
persisted.isSidebarVisible,
|
||||
currentState.isSidebarVisible
|
||||
),
|
||||
mainWindowSize: normalizeMainWindowSize(persisted.mainWindowSize)
|
||||
?? currentState.mainWindowSize,
|
||||
appFontSize: isAllowedSetting(APP_FONT_SIZE_VALUES, persisted.appFontSize)
|
||||
@@ -988,15 +1007,16 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
torrentBindAddress: typeof persisted.torrentBindAddress === 'string'
|
||||
? persisted.torrentBindAddress
|
||||
: currentState.torrentBindAddress,
|
||||
aria2DiskCache: typeof persisted.aria2DiskCache === 'string'
|
||||
? persisted.aria2DiskCache
|
||||
: currentState.aria2DiskCache,
|
||||
aria2DiskCache: persistedString(persisted.aria2DiskCache, currentState.aria2DiskCache),
|
||||
customUserAgent: persistedString(persisted.customUserAgent, currentState.customUserAgent),
|
||||
sidebarPosition: isAllowedSetting(SIDEBAR_POSITION_VALUES, persisted.sidebarPosition)
|
||||
? persisted.sidebarPosition
|
||||
: currentState.sidebarPosition,
|
||||
proxyMode: isAllowedSetting(PROXY_MODE_VALUES, persisted.proxyMode)
|
||||
? persisted.proxyMode
|
||||
: currentState.proxyMode,
|
||||
proxyHost: persistedString(persisted.proxyHost, currentState.proxyHost),
|
||||
proxyPort: persistedFiniteInteger(persisted.proxyPort, 1, 65_535, currentState.proxyPort),
|
||||
mediaCookieSource: isAllowedSetting(MEDIA_COOKIE_SOURCE_VALUES, persisted.mediaCookieSource)
|
||||
? persisted.mediaCookieSource
|
||||
: 'none',
|
||||
@@ -1078,15 +1098,23 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
currentState.adaptiveMirrorSelection
|
||||
),
|
||||
speedLimitPresetValues: Array.isArray(persisted.speedLimitPresetValues)
|
||||
? persisted.speedLimitPresetValues
|
||||
? persisted.speedLimitPresetValues.filter(
|
||||
(value): value is number => typeof value === 'number' && Number.isFinite(value)
|
||||
)
|
||||
: currentState.speedLimitPresetValues,
|
||||
lastCustomSpeedLimitKiB: persistedFiniteInteger(
|
||||
persisted.lastCustomSpeedLimitKiB,
|
||||
1,
|
||||
10_485_760,
|
||||
currentState.lastCustomSpeedLimitKiB
|
||||
),
|
||||
lastCustomSpeedLimitUnit: persisted.lastCustomSpeedLimitUnit === 'KB/s'
|
||||
|| persisted.lastCustomSpeedLimitUnit === 'MB/s'
|
||||
? persisted.lastCustomSpeedLimitUnit
|
||||
: currentState.lastCustomSpeedLimitUnit,
|
||||
logsEnabled: persisted.logsEnabled === true,
|
||||
approvedDownloadRoots: Array.isArray(persisted.approvedDownloadRoots)
|
||||
? persisted.approvedDownloadRoots
|
||||
? persisted.approvedDownloadRoots.filter((root): root is string => typeof root === 'string')
|
||||
: currentState.approvedDownloadRoots,
|
||||
scheduler: {
|
||||
...currentState.scheduler,
|
||||
|
||||
Reference in New Issue
Block a user