fix: enhance NAR URL normalization and fix potential path traversal [backport #856]

[kalbasit]

The previous implementation of Normalize() was vulnerable to path traversal if the NAR hash contained ".." or other malicious sequences. This change refactors Normalize() to use stricter regular expressions for hash validation and returns an error if the hash is invalid.

Additionally, it refactors ensureNarFile in pkg/cache/cache.go to use a new helper createOrUpdateNarFile, reducing code duplication.

Key changes:

• Introduced narHashLenientRegexp and narNormalizedHashRegexp for robust hash validation.
• Modified nar.URL.Normalize() to return (URL, error).
• Updated all callers in pkg/server, pkg/storage, and testdata to handle the new error return.
• Added filepath.Base() to temp file creation in pkg/cache/cache.go for enhanced security.

(cherry picked from commit e3119d4)

[gemini-code-assist]

Summary of Changes

Hello @kalbasit, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the security and robustness of NAR URL handling by addressing potential path traversal vulnerabilities. The Normalize() function for NAR URLs has been refactored to employ stricter hash validation using regular expressions and now explicitly returns an error for invalid inputs. This change propagates error handling throughout the codebase, ensuring that malformed or malicious NAR URLs are properly rejected, thereby preventing security exploits and improving overall system stability.

Highlights

• Enhanced NAR URL Normalization: The Normalize() function for NAR URLs has been refactored to prevent path traversal vulnerabilities and now returns an error for invalid hashes.
• Stricter Hash Validation: New regular expressions (narHashLenientRegexp, narNormalizedHashRegexp) were introduced for more robust and strict validation of NAR hashes.
• Error Handling Propagation: All relevant call sites in pkg/server, pkg/storage, and testdata have been updated to properly handle errors returned by the nar.URL.Normalize() function.
• Improved Temporary File Security: Temporary file creation in pkg/cache/cache.go now uses filepath.Base() on NAR hashes, enhancing security against malicious path inputs.

Changelog

• pkg/cache/cache.go
  • Imported path/filepath.
  • Modified createTempNarFile to use filepath.Base() for generating temporary file patterns, enhancing security.
  • Updated storeInDatabase and storeNarInfoInDatabase to handle errors returned by narURL.Normalize().
• pkg/nar/hash.go
  • Introduced HashPatternLenient and narNormalizedHashRegexp for more granular and robust hash validation.
  • Updated HashPattern to use non-capturing groups for the optional prefix.
• pkg/nar/url.go
  • Removed unused regexp import and hashValidationRegexp.
  • Modified ParseURL to delegate hash validation to ValidateHash.
  • Refactored Normalize() to return (URL, error), implementing stricter validation using the newly defined regexps and returning ErrInvalidHash for invalid inputs.
• pkg/nar/url_test.go
  • Updated TestNormalize to include an errStr field in test cases for expected errors.
  • Modified test assertions to check for expected errors or successful normalization.
  • Adjusted test inputs and outputs to reflect the new strict hash validation.
• pkg/server/server.go
  • Updated getNarInfo to handle the error returned by narURL.Normalize(), logging the error and returning an http.StatusInternalServerError.
• pkg/storage/local/local.go
  • Modified HasNar, GetNar, PutNar, and DeleteNar methods to correctly handle errors returned by narURL.Normalize().
• pkg/storage/s3/s3.go
  • Updated narPath to handle errors returned by narURL.Normalize().
• testdata/server.go
  • Modified the handler function to handle errors returned by nar.URL.Normalize(), returning an http.StatusBadRequest for invalid NAR URLs.

Activity

• No specific activity (comments, reviews, progress updates) was provided in the context for this pull request.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[dosubot]

Related Documentation

Checked 7 published document(s) in 1 knowledge base(s). No updates required.

^{How did I do? Any feedback?}  

[gemini-code-assist]

Code Review

This pull request effectively addresses a path traversal vulnerability in nar.URL.Normalize(). The changes are robust, replacing manual string manipulation with stricter regular expression-based validation. The function signature was correctly updated to return an error, and all call sites have been diligently updated to handle this new error case. Additionally, the use of filepath.Base() when creating temporary files adds a valuable layer of defense-in-depth. The accompanying test changes are thorough and correctly validate the new behavior, including the error conditions. Overall, this is an excellent and well-executed security fix.