Skip to content

fix: strip line-break characters from hostnames and comments#90

Merged
cjimti merged 1 commit into
masterfrom
fix/comment-newline-sanitization
May 30, 2026
Merged

fix: strip line-break characters from hostnames and comments#90
cjimti merged 1 commit into
masterfrom
fix/comment-newline-sanitization

Conversation

@cjimti
Copy link
Copy Markdown
Member

@cjimti cjimti commented May 30, 2026

What

Strip \r, \n, \v, \f, and NUL from hostnames and comments before they are written into the hosts file. Tabs, spaces, and # are preserved, so normal comments (added by kubefwd) are unaffected.

Why

Comments and hostnames passed to AddHost / AddHostWithComment were trimmed but embedded line-break characters were kept. Because lineFormatter interpolates these fields with %s, an embedded \r\n let a single logical entry render as multiple physical lines, corrupting the hosts file structure when it is read back by txeh or any other consumer (CWE-117, improper output neutralization).

The guard is applied in two places:

  • Input boundary (addHostWithComment) so the in-memory model stays clean.
  • Render-time backstop (lineFormatter) so no code path can emit a multi-line entry, even when a HostFileLine is populated directly.

On the report this came from

This was prompted by a "CRLF injection" report claiming privilege escalation (CVSS 6.7 / 8.8). That severity does not hold: every PoC requires root plus an attacker-controlled -w path, which already grants arbitrary file write without any comment trick. The restricted-sudo escalation is a sudoers misconfiguration, since -w alone hands the user root file write.

What is real is the data-hygiene gap fixed here: a library that writes structured files should not let one logical entry silently become several lines. Low severity, no CVE warranted.

Compatibility

The public API is unchanged. Input is stripped silently rather than rejected, so existing callers (including kubefwd) keep working without an error-handling change.

Tests

crlf_test.go adds:

  • table-driven stripLineBreaks unit tests with hardcoded expected values
  • the sudoers PoC payload as a regression test (asserts exactly one physical line)
  • a hostname-CRLF case
  • a lineFormatter backstop test

go test -race ./... passes, coverage 94.8% (gate 80%).

Comments and hostnames passed to AddHost/AddHostWithComment were trimmed
but embedded carriage return, line feed, vertical tab, form feed, and NUL
characters were preserved. Because lineFormatter interpolates these fields
with %s, an embedded "\r\n" let a single logical entry render as multiple
physical lines, corrupting the hosts file structure when read back by txeh
or any other consumer (CWE-117, improper output neutralization).

Add stripLineBreaks and apply it at the input boundary (addHostWithComment)
and as a render-time backstop (lineFormatter) so no code path can emit a
multi-line entry. Tabs, spaces, and # are preserved, so normal comments are
unaffected.

This is a defense-in-depth robustness fix for downstream library callers
that may pass untrusted input. It is not the privilege-escalation vector
described in the report: exploiting the CLI requires root and an
attacker-controlled -w path, which already grants arbitrary file write
without any comment trick. The public API is unchanged; input is stripped
silently rather than rejected to preserve backward compatibility.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.70%. Comparing base (d324339) to head (2d9394a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #90      +/-   ##
==========================================
+ Coverage   89.51%   89.70%   +0.18%     
==========================================
  Files          18       18              
  Lines         706      719      +13     
==========================================
+ Hits          632      645      +13     
  Misses         64       64              
  Partials       10       10              

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cjimti cjimti merged commit 9ccd8b7 into master May 30, 2026
9 checks passed
@cjimti cjimti deleted the fix/comment-newline-sanitization branch May 30, 2026 20:57
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