Skip to content

Conversation

odaysec
Copy link

@odaysec odaysec commented Oct 13, 2025

Summary of Fixes

This pull request addresses multiple security-related issues in the cilium-cli and pkg/policy components that could potentially expose the system to path traversal and string injection vulnerabilities. The changes improve input validation, ensure safe path handling, and replace unsafe string concatenation with proper encoding and marshaling.

1. Path Traversal Prevention in Archive Extraction

File: cilium-cli/sysdump/sysdump.go (around line 2346)

When extracting files from an archive, unvalidated filenames can lead to directory traversal attacks (e.g., entries like ../evil-file). Such attacks may allow files to be written outside the intended extraction directory, potentially overwriting sensitive files.

Fix implemented:

  • The extraction path is now cleaned using filepath.Clean and verified to ensure:

    • It is not an absolute path.
    • It does not contain .. path components.
    • The final resolved path remains within the target directory (dst).
  • Suspicious entries are skipped, and a warning can be logged to indicate skipped entries.

This ensures all files extracted from a zip or tar archive remain confined to the intended directory, preventing arbitrary file write vulnerabilities.

2. Safe JSON Construction in Deployment Annotations

File: cilium-cli/connectivity/check/deployment.go (lines 1903–1906)

The original implementation manually constructed a JSON string using string concatenation that included user-provided values. This approach is unsafe because unescaped quotes or special characters could break the JSON structure or lead to injection vulnerabilities.

Fix implemented:

  • Replaced manual string concatenation with a safer approach using json.Marshal.
  • The annotations are now built into a map[string]annotations and serialized to JSON before being passed to Set.
  • Added proper error handling for json.Marshal and Set.

This change ensures the generated JSON is syntactically valid and eliminates the risk of injection via crafted annotation names or values.

3. Escaping Dangerous Characters in Error Messages

File: pkg/policy/l4.go (around line 1210)

A similar issue was found where an error message was manually embedded in a quoted string without escaping. If the error message contained quotes or control characters, it could break the JSON structure.

Fix implemented:

  • Replaced:

    b = []byte("\"L4Filter error: " + err.Error() + "\"")

    with:

    b = []byte(strconv.Quote("L4Filter error: " + err.Error()))
  • This ensures that all dangerous characters are escaped correctly, and the resulting string is a valid JSON string literal.

  • CWE-22: Improper Limitation of a Pathname to a Restricted Directory (‘Path Traversal’)

  • CWE-116: Improper Encoding or Escaping of Output

  • CWE-79 / CWE-20 (related): Improper Input Validation or Injection via Unescaped Strings


Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

Fixes: #issue-number

<!-- Enter the release note text here if needed or remove this section! -->

@odaysec
Copy link
Author

odaysec commented Oct 13, 2025

I’ve updated the patch for this feature in my project at Fixes: cilium@240441f and released into cilium:main It seems that Roblox may have missed some important information shared by our team at Cilium.

This pull request continues the update to ensure the latest changes are properly synchronized and all relevant improvements are included.

…jection in String Construction

Signed-off-by: Jörmungandrk <[email protected]>
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.

2 participants