Fix validate user-supplied paths and strengthen Input handling lead Uncontrolled data used in path expression #1209
+127
−91
Conversation
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
orangecurl
approved these changes
Sep 18, 2025
|
@odaysec thanks, can you please fix the testing? |
ije
reviewed
Sep 19, 2025
poratoes
approved these changes
Sep 19, 2025
fix the problem, implement robust validation for user-controlled segments of the path (namely `zoneId` in `NpmRC`). - In the `StoreDir()` method, after constructing the proposed directory path, normalize it (using `filepath.Clean`) and ensure it’s a subpath of the configured working directory (`config.WorkDir`). - If validation fails (i.e., the resulting path falls outside of `config.WorkDir`), either return an error or default to a safe directory. - This change should occur in `server/npmrc.go`, specifically within or around the `StoreDir()` method, since this is the central place where user-controlled data enters the filesystem path. This fix does not require new methods or complex flow tracking—just robust application of path normalization and containment checks in the StoreDir logic, possibly switching from `path` to `path/filepath` for correct OS path handling.
This PR updates zoneId validation in `NewNpmRcFromJSON`: - Removed regexp.MustCompile usage - Replaced with `valid.IsDomain(rc.zoneId)` for better performance - Removed unused regexp import Thanks @ije for the suggestion!
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Accessing files using paths constructed from user-controlled data can allow an attacker to access unexpected resources. This can result in sensitive information being revealed or deleted, or an attacker being able to influence behavior by modifying unexpected files. Paths that are naively constructed from data controlled by a user may be absolute paths, or may contain unexpected special characters such as "..". Such a path could point anywhere on the file system.
Description
This PR fixes multiple path traversal vulnerabilities and strengthens input validation to ensure safe filesystem access.
web/handler.go(parseImportMap)AppDirand user-supplied paths before file access.AppDir."Invalid file name"for invalid input.server/npmrc.go(NewNpmRcFromJSON)zoneIdto allow only alphanumeric characters, dashes, and underscores (^[\w\-]+$).server/build_resolver.go(validateJSFile)ctx.wd/node_modules/pkgName).internal/storage/storage_fs.goStat,Get,Put,Delete,DeleteAll, andListto use the validated paths.Notes
path/filepath,strings,regexp).