Skip to content

Commit d1b04b4

Browse files
geoffw0geoffw0
committed
C++: Use asDefiningArgument() where appropriate.
1 parent 9ebdb2a commit d1b04b4

File tree

6 files changed

+36
-93
lines changed

6 files changed

+36
-93
lines changed

‎cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ import SystemData
2121
class ExposedSystemDataConfiguration extends TaintTracking::Configuration {
2222
ExposedSystemDataConfiguration() { this = "ExposedSystemDataConfiguration" }
2323

24-
override predicate isSource(DataFlow::Node source) {
25-
source.asConvertedExpr() = any(SystemData sd).getAnExpr()
26-
}
24+
override predicate isSource(DataFlow::Node source) { source = any(SystemData sd).getAnExpr() }
2725

2826
override predicate isSink(DataFlow::Node sink) {
2927
exists(FunctionCall fc, FunctionInput input, int arg |

‎cpp/ql/src/Security/CWE/CWE-497/PotentiallyExposedSystemData.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class PotentiallyExposedSystemDataConfiguration extends TaintTracking::Configura
3535
PotentiallyExposedSystemDataConfiguration() { this = "PotentiallyExposedSystemDataConfiguration" }
3636

3737
override predicate isSource(DataFlow::Node source) {
38-
source.asExpr() = any(SystemData sd | sd.isSensitive()).getAnExpr()
38+
source = any(SystemData sd | sd.isSensitive()).getAnExpr()
3939
}
4040

4141
override predicate isSink(DataFlow::Node sink) {

‎cpp/ql/src/Security/CWE/CWE-497/SystemData.qll

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import cpp
66
import semmle.code.cpp.commons.Environment
7+
import semmle.code.cpp.ir.dataflow.TaintTracking
78

89
/**
910
* An element that should not be exposed to an adversary.
@@ -12,7 +13,7 @@ abstract class SystemData extends Element {
1213
/**
1314
* Gets an expression that is part of this `SystemData`.
1415
*/
15-
abstract Expr getAnExpr();
16+
abstract DataFlow::Node getAnExpr();
1617

1718
/**
1819
* Holds if this system data is considered especially sensitive (for example
@@ -33,7 +34,7 @@ class EnvData extends SystemData {
3334
.regexpMatch(".*(user|host|admin|root|home|path|http|ssl|snmp|sock|port|proxy|pass|token|crypt|key).*")
3435
}
3536

36-
override Expr getAnExpr() { result = this }
37+
override DataFlow::Node getAnExpr() { result.asConvertedExpr() = this }
3738

3839
override predicate isSensitive() {
3940
this.(EnvironmentRead)
@@ -49,7 +50,7 @@ class EnvData extends SystemData {
4950
class SQLClientInfo extends SystemData {
5051
SQLClientInfo() { this.(FunctionCall).getTarget().hasName("mysql_get_client_info") }
5152

52-
override Expr getAnExpr() { result = this }
53+
override DataFlow::Node getAnExpr() { result.asConvertedExpr() = this }
5354

5455
override predicate isSensitive() { any() }
5556
}
@@ -68,21 +69,24 @@ private predicate sqlConnectInfo(FunctionCall source, Expr use) {
6869
class SQLConnectInfo extends SystemData {
6970
SQLConnectInfo() { sqlConnectInfo(this, _) }
7071

71-
override Expr getAnExpr() { sqlConnectInfo(this, result) }
72+
override DataFlow::Node getAnExpr() { sqlConnectInfo(this, result.asConvertedExpr()) }
7273

7374
override predicate isSensitive() { any() }
7475
}
7576

76-
private predicate posixSystemInfo(FunctionCall source, Element use) {
77+
private predicate posixSystemInfo(FunctionCall source, DataFlow::Node use) {
7778
// size_t confstr(int name, char *buf, size_t len)
7879
// - various OS / system strings, such as the libc version
7980
// int statvfs(const char *__path, struct statvfs *__buf)
8081
// int fstatvfs(int __fd, struct statvfs *__buf)
82+
source.getTarget().hasName(["confstr", "statvfs", "fstatvfs"]) and
83+
use.asDefiningArgument() = source.getArgument(1)
84+
or
8185
// - various filesystem parameters
8286
// int uname(struct utsname *buf)
8387
// - OS name and version
84-
source.getTarget().hasName(["confstr", "statvfs", "fstatvfs", "uname"]) and
85-
use = source.getArgument(1)
88+
source.getTarget().hasName("uname") and
89+
use.asDefiningArgument() = source.getArgument(0)
8690
}
8791

8892
/**
@@ -91,10 +95,10 @@ private predicate posixSystemInfo(FunctionCall source, Element use) {
9195
class PosixSystemInfo extends SystemData {
9296
PosixSystemInfo() { posixSystemInfo(this, _) }
9397

94-
override Expr getAnExpr() { posixSystemInfo(this, result) }
98+
override DataFlow::Node getAnExpr() { posixSystemInfo(this, result) }
9599
}
96100

97-
private predicate posixPWInfo(FunctionCall source, Element use) {
101+
private predicate posixPWInfo(FunctionCall source, DataFlow::Node use) {
98102
// struct passwd *getpwnam(const char *name);
99103
// struct passwd *getpwuid(uid_t uid);
100104
// struct passwd *getpwent(void);
@@ -104,7 +108,7 @@ private predicate posixPWInfo(FunctionCall source, Element use) {
104108
source
105109
.getTarget()
106110
.hasName(["getpwnam", "getpwuid", "getpwent", "getgrnam", "getgrgid", "getgrent"]) and
107-
use = source
111+
use.asConvertedExpr() = source
108112
or
109113
// int getpwnam_r(const char *name, struct passwd *pwd,
110114
// char *buf, size_t buflen, struct passwd **result);
@@ -115,14 +119,20 @@ private predicate posixPWInfo(FunctionCall source, Element use) {
115119
// int getgrnam_r(const char *name, struct group *grp,
116120
// char *buf, size_t buflen, struct group **result);
117121
source.getTarget().hasName(["getpwnam_r", "getpwuid_r", "getgrgid_r", "getgrnam_r"]) and
118-
use = source.getArgument([1, 2, 4])
122+
(
123+
use.asConvertedExpr() = source.getArgument([1, 2]) or
124+
use.asDefiningArgument() = source.getArgument(4)
125+
)
119126
or
120127
// int getpwent_r(struct passwd *pwd, char *buffer, size_t bufsize,
121128
// struct passwd **result);
122129
// int getgrent_r(struct group *gbuf, char *buf,
123130
// size_t buflen, struct group **gbufp);
124131
source.getTarget().hasName(["getpwent_r", "getgrent_r"]) and
125-
use = source.getArgument([0, 1, 3])
132+
(
133+
use.asConvertedExpr() = source.getArgument([0, 1]) or
134+
use.asDefiningArgument() = source.getArgument(3)
135+
)
126136
}
127137

128138
/**
@@ -131,15 +141,15 @@ private predicate posixPWInfo(FunctionCall source, Element use) {
131141
class PosixPWInfo extends SystemData {
132142
PosixPWInfo() { posixPWInfo(this, _) }
133143

134-
override Expr getAnExpr() { posixPWInfo(this, result) }
144+
override DataFlow::Node getAnExpr() { posixPWInfo(this, result) }
135145

136146
override predicate isSensitive() { any() }
137147
}
138148

139-
private predicate windowsSystemInfo(FunctionCall source, Element use) {
149+
private predicate windowsSystemInfo(FunctionCall source, DataFlow::Node use) {
140150
// DWORD WINAPI GetVersion(void);
141151
source.getTarget().hasGlobalName("GetVersion") and
142-
use = source
152+
use.asConvertedExpr() = source
143153
or
144154
// BOOL WINAPI GetVersionEx(_Inout_ LPOSVERSIONINFO lpVersionInfo);
145155
// void WINAPI GetSystemInfo(_Out_ LPSYSTEM_INFO lpSystemInfo);
@@ -149,7 +159,7 @@ private predicate windowsSystemInfo(FunctionCall source, Element use) {
149159
.hasGlobalName([
150160
"GetVersionEx", "GetVersionExA", "GetVersionExW", "GetSystemInfo", "GetNativeSystemInfo"
151161
]) and
152-
use = source.getArgument(0)
162+
use.asDefiningArgument() = source.getArgument(0)
153163
}
154164

155165
/**
@@ -158,7 +168,7 @@ private predicate windowsSystemInfo(FunctionCall source, Element use) {
158168
class WindowsSystemInfo extends SystemData {
159169
WindowsSystemInfo() { windowsSystemInfo(this, _) }
160170

161-
override Expr getAnExpr() { windowsSystemInfo(this, result) }
171+
override DataFlow::Node getAnExpr() { windowsSystemInfo(this, result) }
162172
}
163173

164174
private predicate windowsFolderPath(FunctionCall source, Element use) {
@@ -217,7 +227,7 @@ private predicate windowsFolderPath(FunctionCall source, Element use) {
217227
class WindowsFolderPath extends SystemData {
218228
WindowsFolderPath() { windowsFolderPath(this, _) }
219229

220-
override Expr getAnExpr() { windowsFolderPath(this, result) }
230+
override DataFlow::Node getAnExpr() { windowsFolderPath(this, result.asDefiningArgument()) }
221231
}
222232

223233
private predicate logonUser(FunctionCall source, VariableAccess use) {
@@ -231,7 +241,7 @@ private predicate logonUser(FunctionCall source, VariableAccess use) {
231241
class LogonUser extends SystemData {
232242
LogonUser() { logonUser(this, _) }
233243

234-
override Expr getAnExpr() { logonUser(this, result) }
244+
override DataFlow::Node getAnExpr() { logonUser(this, result.asConvertedExpr()) }
235245

236246
override predicate isSensitive() { any() }
237247
}
@@ -313,7 +323,7 @@ private predicate regQuery(FunctionCall source, TRegQueryParameter param) {
313323
class RegQuery extends SystemData {
314324
RegQuery() { regQuery(this, _) }
315325

316-
override Expr getAnExpr() { regQuery(this, TReturnData(result)) }
326+
override DataFlow::Node getAnExpr() { regQuery(this, TReturnData(result.asDefiningArgument())) }
317327

318328
override predicate isSensitive() {
319329
exists(Expr e |

‎cpp/ql/test/query-tests/Security/CWE/CWE-497/SAMATE/PotentiallyExposedSystemData.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
edges
2-
| tests.c:57:21:57:28 | array to pointer conversion | tests.c:70:70:70:77 | array to pointer conversion |
32
| tests.c:57:21:57:28 | password | tests.c:70:70:70:77 | array to pointer conversion |
43
nodes
5-
| tests.c:57:21:57:28 | array to pointer conversion | semmle.label | array to pointer conversion |
64
| tests.c:57:21:57:28 | password | semmle.label | password |
75
| tests.c:70:70:70:77 | array to pointer conversion | semmle.label | array to pointer conversion |
86
subpaths
97
#select
10-
| tests.c:70:70:70:77 | array to pointer conversion | tests.c:57:21:57:28 | array to pointer conversion | tests.c:70:70:70:77 | array to pointer conversion | This operation potentially exposes sensitive system data from $@. | tests.c:57:21:57:28 | array to pointer conversion | array to pointer conversion |
118
| tests.c:70:70:70:77 | array to pointer conversion | tests.c:57:21:57:28 | password | tests.c:70:70:70:77 | array to pointer conversion | This operation potentially exposes sensitive system data from $@. | tests.c:57:21:57:28 | password | password |

‎cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/ExposedSystemData.expected

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ edges
2020
| tests_sockets.cpp:63:15:63:20 | call to getenv | tests_sockets.cpp:76:19:76:22 | path |
2121
| tests_sockets.cpp:63:15:63:20 | call to getenv | tests_sockets.cpp:80:20:80:23 | (const void *)... |
2222
| tests_sockets.cpp:63:15:63:20 | call to getenv | tests_sockets.cpp:80:20:80:23 | path |
23-
| tests_sysconf.cpp:36:21:36:27 | pathbuf | tests_sysconf.cpp:39:19:39:25 | (const void *)... |
24-
| tests_sysconf.cpp:36:21:36:27 | pathbuf | tests_sysconf.cpp:39:19:39:25 | pathbuf |
23+
| tests_sysconf.cpp:36:21:36:27 | confstr output argument | tests_sysconf.cpp:39:19:39:25 | (const void *)... |
24+
| tests_sysconf.cpp:36:21:36:27 | confstr output argument | tests_sysconf.cpp:39:19:39:25 | pathbuf |
2525
nodes
2626
| tests2.cpp:63:13:63:18 | call to getenv | semmle.label | call to getenv |
2727
| tests2.cpp:63:13:63:18 | call to getenv | semmle.label | call to getenv |
@@ -58,7 +58,7 @@ nodes
5858
| tests_sockets.cpp:76:19:76:22 | path | semmle.label | path |
5959
| tests_sockets.cpp:80:20:80:23 | (const void *)... | semmle.label | (const void *)... |
6060
| tests_sockets.cpp:80:20:80:23 | path | semmle.label | path |
61-
| tests_sysconf.cpp:36:21:36:27 | pathbuf | semmle.label | pathbuf |
61+
| tests_sysconf.cpp:36:21:36:27 | confstr output argument | semmle.label | confstr output argument |
6262
| tests_sysconf.cpp:39:19:39:25 | (const void *)... | semmle.label | (const void *)... |
6363
| tests_sysconf.cpp:39:19:39:25 | pathbuf | semmle.label | pathbuf |
6464
subpaths
@@ -77,4 +77,4 @@ subpaths
7777
| tests_sockets.cpp:43:20:43:23 | path | tests_sockets.cpp:26:15:26:20 | call to getenv | tests_sockets.cpp:43:20:43:23 | path | This operation exposes system data from $@. | tests_sockets.cpp:26:15:26:20 | call to getenv | call to getenv |
7878
| tests_sockets.cpp:76:19:76:22 | path | tests_sockets.cpp:63:15:63:20 | call to getenv | tests_sockets.cpp:76:19:76:22 | path | This operation exposes system data from $@. | tests_sockets.cpp:63:15:63:20 | call to getenv | call to getenv |
7979
| tests_sockets.cpp:80:20:80:23 | path | tests_sockets.cpp:63:15:63:20 | call to getenv | tests_sockets.cpp:80:20:80:23 | path | This operation exposes system data from $@. | tests_sockets.cpp:63:15:63:20 | call to getenv | call to getenv |
80-
| tests_sysconf.cpp:39:19:39:25 | pathbuf | tests_sysconf.cpp:36:21:36:27 | pathbuf | tests_sysconf.cpp:39:19:39:25 | pathbuf | This operation exposes system data from $@. | tests_sysconf.cpp:36:21:36:27 | pathbuf | pathbuf |
80+
| tests_sysconf.cpp:39:19:39:25 | pathbuf | tests_sysconf.cpp:36:21:36:27 | confstr output argument | tests_sysconf.cpp:39:19:39:25 | pathbuf | This operation exposes system data from $@. | tests_sysconf.cpp:36:21:36:27 | confstr output argument | confstr output argument |

0 commit comments

Comments
 (0)