Skip to content

Commit 5a2ce22

Browse files
committed
Check that all bits are set when checking for a flag
The `O_...` macro definitions somtimes set multiple bits, while the bits individually represent the values of different `O_...` macros. This lead to false postives on codebases built against Musl libc, which defines `O_TMPFILE` as `020200000` and `O_DIRECTORY` as `0200000`.
1 parent aa46513 commit 5a2ce22

File tree

2 files changed

+11
-5
lines changed

2 files changed

+11
-5
lines changed

cpp/ql/src/Security/CWE/CWE-732/DoNotCreateWorldWritable.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@ import FilePermissions
1515

1616
predicate worldWritableCreation(FileCreationExpr fc, int mode) {
1717
mode = localUmask(fc).mask(fc.getMode()) and
18-
sets(mode, UnixConstants::s_iwoth())
18+
setsAnyBits(mode, UnixConstants::s_iwoth())
1919
}
2020

2121
predicate setWorldWritable(FunctionCall fc, int mode) {
2222
fc.getTarget().getName() = ["chmod", "fchmod", "_chmod", "_wchmod"] and
2323
mode = fc.getArgument(1).getValue().toInt() and
24-
sets(mode, UnixConstants::s_iwoth())
24+
setsAnyBits(mode, UnixConstants::s_iwoth())
2525
}
2626

2727
from Expr fc, int mode, string message

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,17 @@ string octalFileMode(int mode) {
4545
else result = "[non-standard mode: decimal " + mode + "]"
4646
}
4747

48+
/**
49+
* Holds if the bitmask `value` sets the bits in `flag`.
50+
*/
51+
bindingset[value, flag]
52+
predicate setsFlag(int value, int flag) { value.bitAnd(flag) = flag }
53+
4854
/**
4955
* Holds if the bitmask `mask` sets any of the bit fields in `fields`.
5056
*/
5157
bindingset[mask, fields]
52-
predicate sets(int mask, int fields) { mask.bitAnd(fields) != 0 }
58+
predicate setsAnyBits(int mask, int fields) { mask.bitAnd(fields) != 0 }
5359

5460
/**
5561
* Gets the value that `fc` sets the umask to, if `fc` is a call to
@@ -116,7 +122,7 @@ class OpenCreationExpr extends FileCreationWithOptionalModeExpr {
116122
OpenCreationExpr() {
117123
this.getTarget().getName() = ["open", "_open", "_wopen"] and
118124
exists(int flag | flag = this.getArgument(1).getValue().toInt() |
119-
sets(flag, o_creat()) or sets(flag, o_tmpfile())
125+
setsFlag(flag, o_creat()) or setsFlag(flag, o_tmpfile())
120126
)
121127
}
122128

@@ -145,7 +151,7 @@ class OpenatCreationExpr extends FileCreationWithOptionalModeExpr {
145151
OpenatCreationExpr() {
146152
this.getTarget().getName() = "openat" and
147153
exists(int flag | flag = this.getArgument(2).getValue().toInt() |
148-
sets(flag, o_creat()) or sets(flag, o_tmpfile())
154+
setsFlag(flag, o_creat()) or setsFlag(flag, o_tmpfile())
149155
)
150156
}
151157

0 commit comments

Comments
 (0)