-
Notifications
You must be signed in to change notification settings - Fork 301
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
Fix - faro/receiver - uncontrolled data used in path expression #3056
Conversation
if err != nil { | ||
return nil, err | ||
} | ||
return os.Stat(securedPath) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 10 days ago
To fix the problem, we need to enhance the validation logic in the securePath
function to ensure that the constructed path does not contain any path traversal sequences and is within the intended directory. Specifically, we should:
- Ensure that the
name
parameter does not contain any path separators or parent directory references. - Validate that the final resolved path is within the
basePath
directory.
-
Copy modified lines R82-R87
@@ -81,2 +81,8 @@ | ||
} | ||
|
||
// Ensure the name does not contain any path separators or parent directory references | ||
if strings.Contains(name, "/") || strings.Contains(name, "\\") || strings.Contains(name, "..") { | ||
return "", fmt.Errorf("invalid file name: %s", name) | ||
} | ||
|
||
cleanedPath := filepath.Clean(name) |
if err != nil { | ||
return nil, err | ||
} | ||
return os.ReadFile(securedPath) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 10 days ago
To fix the problem, we need to ensure that the paths derived from user input are properly sanitized and validated. Specifically, we should:
- Ensure that the
securePath
method always checks that the resolved path is within thebasePath
, even if thebasePath
is empty. - Improve the
cleanFilePathPart
function to remove any remaining potentially dangerous characters. - Add additional validation to ensure that the
release
string does not contain any path traversal sequences.
-
Copy modified line R85 -
Copy modified lines R386-R390
@@ -84,3 +84,3 @@ | ||
if fs.basePath == "" { | ||
return cleanedPath, nil | ||
fs.basePath = "." | ||
} | ||
@@ -385,3 +385,7 @@ | ||
func cleanFilePathPart(x string) string { | ||
return strings.TrimLeft(strings.ReplaceAll(strings.ReplaceAll(x, "\\", ""), "/", ""), ".") | ||
cleaned := strings.ReplaceAll(strings.ReplaceAll(x, "\\", ""), "/", "") | ||
if strings.Contains(cleaned, "..") { | ||
return "" | ||
} | ||
return strings.TrimLeft(cleaned, ".") | ||
} |
PR Description
Which issue(s) this PR fixes
Notes to the Reviewer
PR Checklist