Skip to content

Harden advanced settings deserialization [2/4] - #404

Open
andrewiethoff wants to merge 2 commits into
gchudov:masterfrom
andrewiethoff:security-audit
Open

Harden advanced settings deserialization [2/4]#404
andrewiethoff wants to merge 2 commits into
gchudov:masterfrom
andrewiethoff:security-audit

Conversation

@andrewiethoff

@andrewiethoff andrewiethoff commented Jul 31, 2026

Copy link
Copy Markdown

Merge order: 2 of 4

Depends on dotnet10-update (#403). Independent of #405 — the two are sibling branches off dotnet10-update and neither contains the other, so they may be merged in either order.

Order PR Branch Depends on
1 #403 dotnet10-update
2 #404 security-audit #403
3 #405 feat/accuraterip-meta-implementation #403
4 #406 fix/net10-build-output-and-assembly-resolution #405

What this does

The advanced settings block is persisted as JSON with TypeNameHandling.Auto, which means the $type discriminators in the stored settings decide which CLR types get instantiated on load. Without a binder, anyone able to write to the settings file can name an arbitrary type and have it constructed during deserialization.

This adds an allow-list ISerializationBinder (AudioSettingsSerializationBinder) and applies it to both the save and the load path in CUEConfig. The allow-list is derived at runtime from the encoder and decoder settings types that a default CUEConfigAdvanced actually registers, so it stays correct as codecs are added or removed rather than being a hand-maintained list. Anything outside it is rejected with a JsonSerializationException in both directions.

Scope

3 files, +198 / −21.

  • CUETools.Processor/CUEConfig.cs — the binder plus its use in Save and Load
  • CUETools/CUETools.TestProcessor/CUEConfigSecurityTest.cs — new tests: LoadRejectsUntrustedJsonTypeNames and SaveAndLoadAllowsRegisteredJsonTypeNames
  • CUETools/CUETools.TestProcessor/CUETools.TestProcessor.csproj — registers the new test file

Note on reviewing this stack

All four PRs target master, because a cross-fork pull request can only target a branch that exists in this repository. Until #403 merges, this diff also contains its commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant