hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-02 04:05:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #7798 from jketema/missing-open-arg

Browse Source 
C++: Add query for missing mode argument in `open`/`openat` calls
This commit is contained in:
Jeroen Ketema
2022-02-07 12:01:44 +01:00
committed by GitHub
parent ac03fab986 b967eaf25d
commit 1f2865c7cc
11 changed files with 190 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cpp/ql/lib/change-notes/2022-02-02-detect-creat-in-openat.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `cpp/world-writable-file-creation` query now only detects `open` and `openat` calls with the `O_CREAT` or `O_TMPFILE` flag.

4
cpp/ql/lib/change-notes/2022-02-02-detect-missing-mode-argument-for-open.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* Added a new query, `cpp/open-call-with-mode-argument`, to detect when `open` or `openat` is called with the `O_CREAT` or `O_TMPFILE` flag but when the `mode` argument is omitted.

4
cpp/ql/lib/semmle/code/cpp/commons/unix/Constants.qll
View File

@@ -11,10 +11,10 @@ import cpp
*/
bindingset[input]
int parseOctal(string input) {
input.charAt(0) = "0" and
input.regexpMatch("0[0-7]+") and
result =
strictsum(int ix |
ix in [0 .. input.length()]
ix in [1 .. input.length()]
|
8.pow(input.length() - (ix + 1)) * input.charAt(ix).toInt()
)

5
cpp/ql/src/Security/CWE/CWE-732/DoNotCreateWorldWritable.ql
View File

@@ -12,17 +12,16 @@
import cpp
import FilePermissions
import semmle.code.cpp.commons.unix.Constants
predicate worldWritableCreation(FileCreationExpr fc, int mode) {
mode = localUmask(fc).mask(fc.getMode()) and
sets(mode, s_iwoth())
setsAnyBits(mode, UnixConstants::s_iwoth())
}
predicate setWorldWritable(FunctionCall fc, int mode) {
fc.getTarget().getName() = ["chmod", "fchmod", "_chmod", "_wchmod"] and
mode = fc.getArgument(1).getValue().toInt() and
sets(mode, s_iwoth())
setsAnyBits(mode, UnixConstants::s_iwoth())
}
from Expr fc, int mode, string message

97
cpp/ql/src/Security/CWE/CWE-732/FilePermissions.qll
View File

@@ -1,5 +1,49 @@
import cpp
import semmle.code.cpp.commons.unix.Constants
import semmle.code.cpp.commons.unix.Constants as UnixConstants
/**
* Gets the number corresponding to the contents of `input` in base-16.
* Note: the first two characters of `input` must be `0x`. For example:
* `parseHex("0x123abc") = 1194684`.
*/
bindingset[input]
int parseHex(string input) {
exists(string lowerCaseInput | lowerCaseInput = input.toLowerCase() |
lowerCaseInput.regexpMatch("0x[0-9a-f]+") and
result =
strictsum(int ix |
ix in [2 .. input.length()]
|
16.pow(input.length() - (ix + 1)) * "0123456789abcdef".indexOf(lowerCaseInput.charAt(ix))
)
)
}
/**
* Gets the value defined by the `O_CREAT` macro if the macro
* exists and if every definition defines the same value.
*/
int o_creat() {
result =
unique(int v |
exists(Macro m | m.getName() = "O_CREAT" |
v = parseHex(m.getBody()) or v = UnixConstants::parseOctal(m.getBody())
)
)
}
/**
* Gets the value defined by the `O_TMPFILE` macro if the macro
* exists and if every definition defines the same value.
*/
int o_tmpfile() {
result =
unique(int v |
exists(Macro m | m.getName() = "O_TMPFILE" |
v = parseHex(m.getBody()) or v = UnixConstants::parseOctal(m.getBody())
)
)
}
bindingset[n, digit]
private string octalDigit(int n, int digit) {
@@ -20,11 +64,17 @@ string octalFileMode(int mode) {
else result = "[non-standard mode: decimal " + mode + "]"
}
/**
* Holds if the bitmask `value` sets the bits in `flag`.
*/
bindingset[value, flag]
predicate setsFlag(int value, int flag) { value.bitAnd(flag) = flag }
/**
* Holds if the bitmask `mask` sets any of the bit fields in `fields`.
*/
bindingset[mask, fields]
predicate sets(int mask, int fields) { mask.bitAnd(fields) != 0 }
predicate setsAnyBits(int mask, int fields) { mask.bitAnd(fields) != 0 }
/**
* Gets the value that `fc` sets the umask to, if `fc` is a call to
@@ -83,16 +133,24 @@ abstract class FileCreationExpr extends FunctionCall {
abstract int getMode();
}
class OpenCreationExpr extends FileCreationExpr {
abstract class FileCreationWithOptionalModeExpr extends FileCreationExpr {
abstract predicate hasModeArgument();
}
class OpenCreationExpr extends FileCreationWithOptionalModeExpr {
OpenCreationExpr() {
this.getTarget().getName() = ["open", "_open", "_wopen"] and
sets(this.getArgument(1).getValue().toInt(), o_creat())
this.getTarget().hasGlobalOrStdName(["open", "_open", "_wopen"]) and
exists(int flag | flag = this.getArgument(1).getValue().toInt() |
setsFlag(flag, o_creat()) or setsFlag(flag, o_tmpfile())
)
}
override Expr getPath() { result = this.getArgument(0) }
override predicate hasModeArgument() { exists(this.getArgument(2)) }
override int getMode() {
if exists(this.getArgument(2))
if this.hasModeArgument()
then result = this.getArgument(2).getValue().toInt()
else
// assume anything is permitted
@@ -108,20 +166,35 @@ class CreatCreationExpr extends FileCreationExpr {
override int getMode() { result = this.getArgument(1).getValue().toInt() }
}
class OpenatCreationExpr extends FileCreationExpr {
class OpenatCreationExpr extends FileCreationWithOptionalModeExpr {
OpenatCreationExpr() {
this.getTarget().getName() = "openat" and
this.getNumberOfArguments() = 4
this.getTarget().hasGlobalOrStdName("openat") and
exists(int flag | flag = this.getArgument(2).getValue().toInt() |
setsFlag(flag, o_creat()) or setsFlag(flag, o_tmpfile())
)
}
override Expr getPath() { result = this.getArgument(1) }
override int getMode() { result = this.getArgument(3).getValue().toInt() }
override predicate hasModeArgument() { exists(this.getArgument(3)) }
override int getMode() {
if this.hasModeArgument()
then result = this.getArgument(3).getValue().toInt()
else
// assume anything is permitted
result = 0.bitNot()
}
}
private int fopenMode() {
result =
s_irusr().bitOr(s_irgrp()).bitOr(s_iroth()).bitOr(s_iwusr()).bitOr(s_iwgrp()).bitOr(s_iwoth())
UnixConstants::s_irusr()
.bitOr(UnixConstants::s_irgrp())
.bitOr(UnixConstants::s_iroth())
.bitOr(UnixConstants::s_iwusr())
.bitOr(UnixConstants::s_iwgrp())
.bitOr(UnixConstants::s_iwoth())
}
class FopenCreationExpr extends FileCreationExpr {
@@ -153,6 +226,6 @@ class FopensCreationExpr extends FileCreationExpr {
// fopen_s has restrictive permissions unless you have "u" in the mode
if this.getArgument(2).getValue().charAt(_) = "u"
then result = fopenMode()
else result = s_irusr().bitOr(s_iwusr())
else result = UnixConstants::s_irusr().bitOr(UnixConstants::s_iwusr())
}
}

9
cpp/ql/src/Security/CWE/CWE-732/OpenCallMissingModeArgument.c Normal file
View File

@@ -0,0 +1,9 @@
int open_file_bad() {
// BAD - this uses arbitrary bytes from the stack as mode argument
return open(FILE, O_CREAT)
}
int open_file_good() {
// GOOD - the mode argument is supplied
return open(FILE, O_CREAT, S_IRUSR | S_IWUSR)
}

31
cpp/ql/src/Security/CWE/CWE-732/OpenCallMissingModeArgument.qhelp Normal file
View File

@@ -0,0 +1,31 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
When opening a file with the <code>O_CREAT</code> or <code>O_TMPFILE</code> flag, the <code>mode</code> must
be supplied. If the <code>mode</code> argument is omitted, some arbitrary bytes from the stack will be used
as the file mode. This leaks some bits from the stack into the permissions of the file.
</p>
</overview>
<recommendation>
<p>
The <code>mode</code> must be supplied when <code>O_CREAT</code> or <code>O_TMPFILE</code> is specified.
</p>
</recommendation>
<example>
<p>
The first example opens a file with the <code>O_CREAT</code> flag without supplying the <code>mode</code>
argument. In this case arbitrary bytes from the stack will be used as <code>mode</code> argument. The
second example correctly supplies the <code>mode</code> argument and creates a file that is user readable
and writable.
</p>
<sample src="OpenCallMissingModeArgument.c" />
</example>
</qhelp>

19
cpp/ql/src/Security/CWE/CWE-732/OpenCallMissingModeArgument.ql Normal file
View File

@@ -0,0 +1,19 @@
/**
* @name File opened with O_CREAT flag but without mode argument
* @description Opening a file with the O_CREAT flag but without mode argument reads arbitrary bytes from the stack.
* @kind problem
* @problem.severity error
* @security-severity 7.8
* @precision high
* @id cpp/open-call-with-mode-argument
* @tags security
* external/cwe/cwe-732
*/
import cpp
import FilePermissions
from FileCreationWithOptionalModeExpr fc
where not fc.hasModeArgument()
select fc,
"A file is created here without providing a mode argument, which may leak bits from the stack."

29
cpp/ql/test/query-tests/Security/CWE/CWE-732/OpenCallMissingModeArgument.c Normal file
View File

@@ -0,0 +1,29 @@
typedef unsigned int mode_t;
#define O_RDWR 0x0002
#define O_CLOEXEC 0x0040
#define O_NONBLOCK 0x0080
#define O_CREAT 0x0200
#define O_APPEND 0x0800
#define O_TMPFILE 0x2000
int open(const char *pathname, int flags, ...);
int openat(int dirfd, const char *pathname, int flags, ...);
const char *a_file = "/a_file";
void test_open() {
open(a_file, O_NONBLOCK); // GOOD
open(a_file, O_RDWR | O_CLOEXEC); // GOOD
open(a_file, O_APPEND); // GOOD
open(a_file, O_CREAT); // BAD
open(a_file, O_CREAT, 0); // GOOD
open(a_file, O_TMPFILE); // BAD
open(a_file, O_TMPFILE, 0); // GOOD
openat(0, a_file, O_APPEND); // GOOD
openat(0, a_file, O_CREAT); // BAD
openat(0, a_file, O_CREAT, 0); // GOOD
openat(0, a_file, O_TMPFILE); // BAD
openat(0, a_file, O_TMPFILE, 0); // GOOD
}

4
cpp/ql/test/query-tests/Security/CWE/CWE-732/OpenCallMissingModeArgument.expected Normal file
View File

@@ -0,0 +1,4 @@
| OpenCallMissingModeArgument.c:20:3:20:6 | call to open | A file is created here without providing a mode argument, which may leak bits from the stack. |
| OpenCallMissingModeArgument.c:22:3:22:6 | call to open | A file is created here without providing a mode argument, which may leak bits from the stack. |
| OpenCallMissingModeArgument.c:25:3:25:8 | call to openat | A file is created here without providing a mode argument, which may leak bits from the stack. |
| OpenCallMissingModeArgument.c:27:3:27:8 | call to openat | A file is created here without providing a mode argument, which may leak bits from the stack. |

1
cpp/ql/test/query-tests/Security/CWE/CWE-732/OpenCallMissingModeArgument.qlref Normal file
View File

@@ -0,0 +1 @@
Security/CWE/CWE-732/OpenCallMissingModeArgument.ql