Skip to content

Commit 5cd6388

Browse files
fdzdevcv
authored andcommitted
fix(security): validate UPSTREAM_DNS before shell interpolation (#1141)
## Summary - Add IPv4 regex validation for `UPSTREAM_DNS` before it is interpolated into the `docker exec kubectl patch` command (CWE-78, NVBUG 6009988) - Without validation, a malicious `resolv.conf` entry could inject shell metacharacters into the kubectl patch JSON payload - Rejects anything that isn't a valid IPv4 dotted-quad, aborting with a clear error ## Test plan - [ ] `fix-coredns.sh` with valid DNS (e.g. `8.8.8.8`) — patches CoreDNS normally - [ ] `fix-coredns.sh` with no resolvable DNS — falls back to `8.8.8.8`, passes validation - [ ] Simulated bad `UPSTREAM_DNS='"; rm -rf /'` — rejected with error, no execution <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Added validation for DNS upstream settings to prevent invalid values from being applied. * Added a precondition check for a required tooling dependency; the script now fails fast with a clear error if it's missing. * Improved how DNS configuration is constructed and applied to reduce risk of malformed updates and restarts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Carlos Villela <[email protected]>
1 parent 5ff0681 commit 5cd6388

File tree

1 file changed

+43
-1
lines changed

1 file changed

+43
-1
lines changed

scripts/fix-coredns.sh

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,51 @@ if [ -z "$UPSTREAM_DNS" ]; then
6565
UPSTREAM_DNS="8.8.8.8"
6666
fi
6767

68+
# Defense-in-depth: reject values with characters that are never valid in
69+
# an IP address or DNS hostname. The real injection fix is the jq-based
70+
# JSON construction below — this just catches obvious garbage early.
71+
if ! printf '%s' "$UPSTREAM_DNS" | grep -qE '^[a-zA-Z0-9.:_-]+$'; then
72+
echo "ERROR: UPSTREAM_DNS='$UPSTREAM_DNS' contains invalid characters. Aborting."
73+
exit 1
74+
fi
75+
76+
if ! command -v jq >/dev/null 2>&1; then
77+
echo "ERROR: jq is required to safely construct the kubectl patch payload."
78+
exit 1
79+
fi
80+
6881
echo "Patching CoreDNS to forward to $UPSTREAM_DNS..."
6982

70-
docker exec "$CLUSTER" kubectl patch configmap coredns -n kube-system --type merge -p "{\"data\":{\"Corefile\":\".:53 {\\n errors\\n health\\n ready\\n kubernetes cluster.local in-addr.arpa ip6.arpa {\\n pods insecure\\n fallthrough in-addr.arpa ip6.arpa\\n }\\n hosts /etc/coredns/NodeHosts {\\n ttl 60\\n reload 15s\\n fallthrough\\n }\\n prometheus :9153\\n cache 30\\n loop\\n reload\\n loadbalance\\n forward . $UPSTREAM_DNS\\n}\\n\"}}" >/dev/null
83+
# Build the Corefile as a plain string, then let jq handle all JSON
84+
# escaping (CWE-78, NVBUG 6009988). This avoids interpolating
85+
# UPSTREAM_DNS into a shell-constructed JSON/string literal.
86+
read -r -d '' COREFILE <<COREFILE_EOF || true
87+
.:53 {
88+
errors
89+
health
90+
ready
91+
kubernetes cluster.local in-addr.arpa ip6.arpa {
92+
pods insecure
93+
fallthrough in-addr.arpa ip6.arpa
94+
}
95+
hosts /etc/coredns/NodeHosts {
96+
ttl 60
97+
reload 15s
98+
fallthrough
99+
}
100+
prometheus :9153
101+
cache 30
102+
loop
103+
reload
104+
loadbalance
105+
forward . ${UPSTREAM_DNS}
106+
}
107+
COREFILE_EOF
108+
109+
PATCH_JSON="$(jq -n --arg corefile "$COREFILE" '{"data":{"Corefile":$corefile}}')"
110+
111+
docker exec "$CLUSTER" kubectl patch configmap coredns -n kube-system \
112+
--type merge -p "$PATCH_JSON" >/dev/null
71113

72114
docker exec "$CLUSTER" kubectl rollout restart deploy/coredns -n kube-system >/dev/null
73115

0 commit comments

Comments
 (0)