mirror of
https://github.com/farion1231/cc-switch.git
synced 2026-07-29 18:05:37 +08:00
Jason
c98913df41
`import_sql_string_inner` validated only that the file starts with the `-- CC Switch SQLite 导出` comment, then handed the whole text to `execute_batch`. Anything after that prefix ran unchecked, so a crafted backup could `ATTACH DATABASE '/path/x.db'` and create a SQLite file anywhere the user can write. The side effect lands before `validate_basic_state`, so the file survives even when the import as a whole fails. `settings` is in neither SYNC_SKIP_TABLES nor SYNC_PRESERVE_TABLES, so the WebDAV/S3 sync path reaches the same code. Install a SQLite authorizer for the duration of the external batch only, then clear it so our own schema maintenance is unaffected. Deny what can leave the temp database rather than allow-listing what `dump_sql` emits. The batch runs on a throwaway NamedTempFile whose entire contents are already decided by that same SQL, so DELETE/DROP/ UPDATE hand an attacker nothing new -- the only meaningful boundary is the temp file itself. A strict allow-list only adds the risk of refusing a legitimate backup whose schema has a shape we did not anticipate. The denied set was measured, not guessed: `ATTACH DATABASE 'x'`, `VACUUM INTO 'x'` and bare `VACUUM` all surface as `AuthAction::Attach`, so one rule covers all three -- which keyword scanning would not, since `VACUUM INTO` contains no "ATTACH". Also deny vtable creation (file-backed modules such as csvfile can read and write arbitrary paths) and `Unknown`, so future SQLite statements fail closed. Tests cover both denied statements (asserting no file is left on disk, not merely that the call errors) and a real export round-trip, which guards against the allow-list regressing into false refusals.