hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #6948 from ihsinme/ihsinme-patch-076

Browse Source 
CPP: Add query for CWE-243 Creation of chroot Jail Without Changing Working Directory
This commit is contained in:
Mathias Vorreiter Pedersen
2021-11-03 18:50:13 +00:00
committed by GitHub
parent d00196f6be aef0275b3c
commit e9b114630a
6 changed files with 165 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.cpp Normal file
View File

@@ -0,0 +1,24 @@
...
chroot("/myFold/myTmp"); // BAD
...
chdir("/myFold/myTmp"); // BAD
...
int fd = open("/myFold/myTmp", O_RDONLY | O_DIRECTORY);
fchdir(fd); // BAD
...
if (chdir("/myFold/myTmp") == -1) {
exit(-1);
}
if (chroot("/myFold/myTmp") == -1) { // GOOD
exit(-1);
}
...
if (chdir("/myFold/myTmp") == -1) { // GOOD
exit(-1);
}
...
int fd = open("/myFold/myTmp", O_RDONLY | O_DIRECTORY);
if(fchdir(fd) == -1) { // GOOD
exit(-1);
}
...

23
cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.qhelp Normal file
View File

@@ -0,0 +1,23 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Working with changing directories, without checking the return value or pinning the directory, may not be safe. Requires the attention of developers.</p>
</overview>
<example>
<p>The following example demonstrates erroneous and corrected work with changing working directories.</p>
<sample src="IncorrectChangingWorkingDirectory.cpp" />
</example>
<references>
<li>
CERT C Coding Standard:
<a href="https://wiki.sei.cmu.edu/confluence/display/c/POS05-C.+Limit+access+to+files+by+creating+a+jail">POS05-C. Limit access to files by creating a jail.</a>
</li>
</references>
</qhelp>

69
cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql Normal file
View File

@@ -0,0 +1,69 @@
/**
* @name Find work with changing working directories, with security errors.
* @description Not validating the return value or pinning the directory can be unsafe.
* @kind problem
* @id cpp/work-with-changing-working-directories
* @problem.severity warning
* @precision medium
* @tags correctness
* security
* external/cwe/cwe-243
* external/cwe/cwe-252
*/
import cpp
import semmle.code.cpp.commons.Exclusions
/** Holds if a `fc` function call is available before or after a `chdir` function call. */
predicate inExistsChdir(FunctionCall fcp) {
exists(FunctionCall fctmp |
(
fctmp.getTarget().hasGlobalOrStdName("chdir") or
fctmp.getTarget().hasGlobalOrStdName("fchdir")
) and
(
fcp.getBasicBlock().getASuccessor*() = fctmp.getBasicBlock() or
fctmp.getBasicBlock().getASuccessor*() = fcp.getBasicBlock()
)
)
}
/** Holds if a `fc` function call is available before or after a function call containing a `chdir` call. */
predicate outExistsChdir(FunctionCall fcp) {
exists(FunctionCall fctmp |
exists(FunctionCall fctmp2 |
(
fctmp2.getTarget().hasGlobalOrStdName("chdir") or
fctmp2.getTarget().hasGlobalOrStdName("fchdir")
) and
// we are looking for a call containing calls chdir and fchdir
fctmp2.getEnclosingStmt().getParentStmt*() = fctmp.getTarget().getEntryPoint().getChildStmt*()
) and
(
fcp.getBasicBlock().getASuccessor*() = fctmp.getBasicBlock() or
fctmp.getBasicBlock().getASuccessor*() = fcp.getBasicBlock()
)
)
}
from FunctionCall fc, string msg
where
fc.getTarget().hasGlobalOrStdName("chroot") and
not inExistsChdir(fc) and
not outExistsChdir(fc) and
// in this section I want to exclude calls to functions containing chroot that have a direct path to chdir, or to a function containing chdir
exists(FunctionCall fctmp |
fc.getEnclosingStmt().getParentStmt*() = fctmp.getTarget().getEntryPoint().getChildStmt*() and
not inExistsChdir(fctmp) and
not outExistsChdir(fctmp)
) and
msg = "Creation of 'chroot' jail without changing the working directory"
or
(
fc.getTarget().hasGlobalOrStdName("chdir") or
fc.getTarget().hasGlobalOrStdName("fchdir")
) and
fc instanceof ExprInVoidContext and
not isFromMacroDefinition(fc) and
msg = "Unchecked return value for call to '" + fc.getTarget().getName() + "'."
select fc, msg

2
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-243/semmle/tests/IncorrectChangingWorkingDirectory.expected Normal file
View File

@@ -0,0 +1,2 @@
| test.cpp:12:7:12:12 | call to chroot | Creation of 'chroot' jail without changing the working directory |
| test.cpp:29:3:29:7 | call to chdir | Unchecked return value for call to 'chdir'. |

1
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-243/semmle/tests/IncorrectChangingWorkingDirectory.qlref Normal file
View File

@@ -0,0 +1 @@
experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql

46
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-243/semmle/tests/test.cpp Normal file
View File

@@ -0,0 +1,46 @@
typedef int FILE;
#define size_t int
size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
FILE *fopen(const char *filename, const char *mode);
int fread(char *buf, int size, int count, FILE *fp);
int fclose(FILE *fp);
int chroot(char *path);
int chdir(char *path);
void exit(int status);
int funTest1(){
if (chroot("/myFold/myTmp") == -1) { // BAD
exit(-1);
}
return 0;
}
int funTest2(){
if (chdir("/myFold/myTmp") == -1) { // GOOD
exit(-1);
}
if (chroot("/myFold/myTmp") == -1) { // GOOD
exit(-1);
}
return 0;
}
int funTest3(){
chdir("/myFold/myTmp"); // BAD
return 0;
}
int main(int argc, char *argv[])
{
if(argc = 0) {
funTest3();
return 2;
}
if(argc = 1)
funTest1();
else
funTest2();
FILE *fp = fopen(argv[1], "w");
fwrite("12345", 5, 1, fp);
fclose(fp);
return 0;
}