fix(net): reject control-char / path-traversal injection in wireguard/ipsec/migrate config fields (1.1.21)#224
Merged
Conversation
…/ipsec/migrate config fields (1.1.21) Three sinks emitted a less-trusted string verbatim into a generated config or used it as a filesystem path without a control-char / traversal check: - WireguardPure::validateConfig never checked the free-text FwMark, DNS, MTU, and peer description fields, all written into the wg-quick .conf. An embedded newline injects a directive — most dangerously `PostUp = <cmd>`, which wg-quick runs as root. New hasControlChar helper rejects any byte < 0x20 or == 0x7f in those fields. - IpsecPure::validateConfig never checked leftid/rightid/description, emitted into ipsec.conf. A newline injects an arbitrary conn option (e.g. rightsubnet=0.0.0.0/0, silently widening tunnel policy). - MigratePure gained validateArtifactFile: the artifact filename comes from the source server's /export JSON and is used as both a `curl -o` and an `unlink()` path. A hostile source returning "../../../etc/cron.d/pwn" got arbitrary file write + delete. It is now validated as a single safe path component before use in lib/migrate.cpp. Adds wireguard/ipsec/migrate pure-test cases for each rejection and a clean-value accept. Bumps version to 1.1.21; CHANGELOG + trust-model (en/uk) updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X6t6tzVypHye5bDGLxzmZK
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
2 participants
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
The pure-module audit found three sinks where a less-trusted string was emitted verbatim into a generated config or used as a filesystem path without a control-character / traversal check. Each is fixed with validation in the pure (unit-tested) layer.
WireGuard —
lib/wireguard_pure.cppvalidateConfigvalidated keys, CIDRs, ports and endpoints, but never the free-textFwMark,DNS,MTU, and peerdescriptionfields — all written straight into the wg-quick.conf. An embedded newline escapes the line and injects its own directive, most dangerouslyPostUp = <cmd>, which wg-quick runs as root. NewhasControlCharhelper rejects any byte< 0x20or== 0x7f(incl.\n/\r) in those fields.IPsec —
lib/ipsec_pure.cppvalidateConfignever checkedleftid/rightid/description, all emitted intoipsec.conf(leftid=…/rightid=…/# …). A newline injects an arbitrary conn option, e.g.rightsubnet=0.0.0.0/0, silently widening the tunnel policy. Control bytes in those three fields are now rejected.migrate —
lib/migrate_pure.{h,cpp}+lib/migrate.cppNew
validateArtifactFile. The artifact filename comes from the source server's/exportJSON (extractFileField) and is concatenated intoworkDir + "/" + name, then used as both acurl -otarget and anunlink()target. A hostile/compromised source returning"../../../etc/cron.d/pwn"got arbitrary file write and delete on the migrating host. The filename is now validated as a single safe path component (no/, no.., no control bytes, ≤ 255 chars) inlib/migrate.cppbefore use.Tests
New
wireguard_pure_test,ipsec_pure_test, andmigrate_pure_testcases cover each rejection (newline / traversal / slash / control byte) and confirm clean values still validate. All three suites pass locally.Version
Bumps to 1.1.21;
CHANGELOG.mdentry anddocs/trust-model.{md,uk.md}Applies-to line updated.🤖 Generated with Claude Code
Generated by Claude Code