-
-
Notifications
You must be signed in to change notification settings - Fork 194
Fix validate user-supplied paths and strengthen Input handling lead Uncontrolled data used in path expression #1209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@odaysec thanks, can you please fix the testing? |
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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
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
)AppDir
and user-supplied paths before file access.AppDir
."Invalid file name"
for invalid input.server/npmrc.go
(NewNpmRcFromJSON
)zoneId
to allow only alphanumeric characters, dashes, and underscores (^[\w\-]+$
).server/build_resolver.go
(validateJSFile
)ctx.wd/node_modules/pkgName
).internal/storage/storage_fs.go
Stat
,Get
,Put
,Delete
,DeleteAll
, andList
to use the validated paths.Notes
path/filepath
,strings
,regexp
).