mirror of
https://github.com/github/codeql.git
synced 2026-04-28 02:05:14 +02:00
updates based on review
This commit is contained in:
@@ -62,7 +62,7 @@ is still contained within that directory.
|
||||
OWASP:
|
||||
<a href="https://owasp.org/www-community/attacks/Path_Traversal">Path Traversal</a>.
|
||||
</li>
|
||||
<li>Rails: <a href="https://api.rubyonrails.org/classes/ActiveStorage/Filename.html#method-i-sanitized">ActiveStorage::Filename#sanitized</a>.</li>
|
||||
<li>Linux man pages: <a href="https://man7.org/linux/man-pages/man3/realpath.3.html">realpath(3)</a>.</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -5,22 +5,24 @@ int main(int argc, char** argv) {
|
||||
char *userAndFile = argv[2];
|
||||
char baseDir[PATH_MAX] = "/home/user/public/";
|
||||
char fullPath[PATH_MAX];
|
||||
char resolvedPath[PATH_MAX];
|
||||
|
||||
// Attempt to concatenate the base directory and the user-supplied path
|
||||
snprintf(fullPath, sizeof(fullPath), "%s%s", baseDir, userAndFile);
|
||||
|
||||
// Resolve the absolute path, normalizing any ".." or "."
|
||||
if (realpath(fullPath, resolvedPath) == NULL) {
|
||||
char *resolvedPath = realpath(fullPath, NULL);
|
||||
if (resolvedPath == NULL) {
|
||||
perror("Error resolving path");
|
||||
return 1;
|
||||
}
|
||||
|
||||
// Check if the resolved path starts with the base directory
|
||||
if (strncmp(baseDir, resolvedPath, strlen(baseDir)) != 0) {
|
||||
free(resolvedPath);
|
||||
return 1;
|
||||
}
|
||||
|
||||
// GOOD: Path is within the intended directory
|
||||
FILE *file = fopen(resolvedPath, "wb+");
|
||||
free(resolvedPath);
|
||||
}
|
||||
@@ -10,9 +10,5 @@ int main(int argc, char** argv) {
|
||||
return 1;
|
||||
}
|
||||
|
||||
char fileBuffer[FILENAME_MAX] = "/home/user/files/";
|
||||
// Ensure buffer overflow is prevented
|
||||
strncat(fileBuffer, userAndFile, FILENAME_MAX - strlen(fileBuffer) - 1);
|
||||
// GOOD: We know that the filename is safe and stays within the public folder
|
||||
FILE *file = fopen(fileBuffer, "wb+");
|
||||
// use `userAndFile` as a safe filename
|
||||
}
|
||||
Reference in New Issue
Block a user