Skip to content

Commit 92d9e51

Browse files
committed
Extract the value of O_CREAT and O_TMPFILE from the defining macro
There are operating systems that define `O_CREAT` with a different value than Linux, which uses `0x40`. For example, OpenBSD uses `0x0200`. Hence, we cannot use a hardcoded value. Also handle `O_TMPFILE` while here.
1 parent bd859d9 commit 92d9e51

File tree

4 files changed

+44
-10
lines changed

4 files changed

+44
-10
lines changed

cpp/ql/src/Security/CWE/CWE-732/FilePermissions.qll

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,29 @@
11
import cpp
22
import semmle.code.cpp.commons.unix.Constants
33

4+
bindingset[input]
5+
int parseHex(string input) {
6+
input.prefix(2) = "0x" and
7+
result =
8+
strictsum(int ix |
9+
ix in [2 .. input.length()]
10+
|
11+
16.pow(input.length() - (ix + 1)) * "0123456789abcdef".indexOf(input.charAt(ix).toLowerCase())
12+
)
13+
}
14+
15+
int o_creat_new() {
16+
exists(Macro m | m.getName() = "O_CREAT" | result = parseHex(m.getBody()))
17+
or
18+
exists(Macro m | m.getName() = "O_CREAT" | result = parseOctal(m.getBody()))
19+
}
20+
21+
int o_tmpfile() {
22+
exists(Macro m | m.getName() = "O_TMPFILE" | result = parseHex(m.getBody()))
23+
or
24+
exists(Macro m | m.getName() = "O_TMPFILE" | result = parseOctal(m.getBody()))
25+
}
26+
427
bindingset[n, digit]
528
private string octalDigit(int n, int digit) {
629
result = n.bitShiftRight(digit * 3).bitAnd(7).toString()
@@ -90,7 +113,9 @@ abstract class FileCreationWithOptionalModeExpr extends FileCreationExpr {
90113
class OpenCreationExpr extends FileCreationWithOptionalModeExpr {
91114
OpenCreationExpr() {
92115
this.getTarget().getName() = ["open", "_open", "_wopen"] and
93-
sets(this.getArgument(1).getValue().toInt(), o_creat())
116+
exists(int flag | flag = this.getArgument(1).getValue().toInt() |
117+
sets(flag, o_creat_new()) or sets(flag, o_tmpfile())
118+
)
94119
}
95120

96121
override Expr getPath() { result = this.getArgument(0) }
@@ -117,7 +142,9 @@ class CreatCreationExpr extends FileCreationExpr {
117142
class OpenatCreationExpr extends FileCreationWithOptionalModeExpr {
118143
OpenatCreationExpr() {
119144
this.getTarget().getName() = "openat" and
120-
sets(this.getArgument(2).getValue().toInt(), o_creat())
145+
exists(int flag | flag = this.getArgument(2).getValue().toInt() |
146+
sets(flag, o_creat_new()) or sets(flag, o_tmpfile())
147+
)
121148
}
122149

123150
override Expr getPath() { result = this.getArgument(1) }

cpp/ql/src/Security/CWE/CWE-732/OpenCallMissingModeArgument.qhelp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,15 @@
55

66
<overview>
77
<p>
8-
When opening a file with the <code>O_CREAT</code> flag, the <code>mode</code> must be supplied. If the
9-
<code>mode</code> argument is omitted, some arbitrary bytes from the stack will be used as the file mode.
10-
This leaks some bits from the stack into the permissions of the file.
8+
When opening a file with the <code>O_CREAT</code> or <code>O_TMPFILE</code> flag, the <code>mode</code> must
9+
be supplied. If the <code>mode</code> argument is omitted, some arbitrary bytes from the stack will be used
10+
as the file mode. This leaks some bits from the stack into the permissions of the file.
1111
</p>
1212
</overview>
1313

1414
<recommendation>
1515
<p>
16-
The <code>mode</code> must be supplied when <code>O_CREAT</code> is specified.
16+
The <code>mode</code> must be supplied when <code>O_CREAT</code> or <code>O_TMPFILE</code> is specified.
1717
</p>
1818
</recommendation>
1919

cpp/ql/test/query-tests/Security/CWE/CWE-732/OpenCallMissingModeArgument.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
typedef unsigned int mode_t;
22

3-
#define O_APPEND 0010
4-
#define O_CREAT 0100
3+
#define O_APPEND 0x0020
4+
#define O_CREAT 0x0200
5+
#define O_TMPFILE 0x2000
56

67
int open(const char *pathname, int flags, ...);
78

@@ -13,7 +14,11 @@ void test_open() {
1314
open(a_file, O_APPEND); // GOOD
1415
open(a_file, O_CREAT); // BAD
1516
open(a_file, O_CREAT, 0); // GOOD
17+
open(a_file, O_TMPFILE); // BAD
18+
open(a_file, O_TMPFILE, 0); // GOOD
1619
openat(0, a_file, O_APPEND); // GOOD
1720
openat(0, a_file, O_CREAT); // BAD
1821
openat(0, a_file, O_CREAT, 0); // GOOD
22+
openat(0, a_file, O_TMPFILE); // BAD
23+
openat(0, a_file, O_TMPFILE, 0); // GOOD
1924
}
Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
1-
| OpenCallMissingModeArgument.c:14:3:14:6 | call to open | A file is created here without providing a mode argument, which may leak bits from the stack. |
2-
| OpenCallMissingModeArgument.c:17:3:17:8 | call to openat | A file is created here without providing a mode argument, which may leak bits from the stack. |
1+
| OpenCallMissingModeArgument.c:15:3:15:6 | call to open | A file is created here without providing a mode argument, which may leak bits from the stack. |
2+
| OpenCallMissingModeArgument.c:17:3:17:6 | call to open | A file is created here without providing a mode argument, which may leak bits from the stack. |
3+
| OpenCallMissingModeArgument.c:20:3:20:8 | call to openat | A file is created here without providing a mode argument, which may leak bits from the stack. |
4+
| OpenCallMissingModeArgument.c:22:3:22:8 | call to openat | A file is created here without providing a mode argument, which may leak bits from the stack. |

0 commit comments

Comments
 (0)