Skip to content

Commit 803cb3f

Browse files
committed
C#: Address review comment
- Flow from expressions with a value is excluded.
1 parent 7caae01 commit 803cb3f

File tree

3 files changed

+8
-6
lines changed

3 files changed

+8
-6
lines changed

csharp/ql/src/Security Features/CWE-020/RuntimeChecksBypass.ql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ Expr uncheckedWrite(Callable callable, Field f) {
3535
result.getEnclosingCallable() = callable and
3636
not callable.calls*(checkedWrite(f, _, _).getEnclosingCallable()) and
3737
// Exclude object creations because they were not deserialized
38-
not exists(ObjectCreation src | DataFlow::localExprFlow(src, result))
38+
not exists(Expr src | DataFlow::localExprFlow(src, result) |
39+
src instanceof ObjectCreation or src.hasValue()
40+
)
3941
}
4042

4143
from BinarySerializableType t, Field f, IfStmt check, Expr write, Expr unsafeWrite

csharp/ql/test/query-tests/Security Features/CWE-020/RuntimeChecksBypass.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public Test1(string v)
1717
[OnDeserializing]
1818
public void Deserialize()
1919
{
20-
f = "invalid"; // BAD
20+
f = $"invalid"; // BAD
2121
}
2222
}
2323

@@ -37,7 +37,7 @@ public Test2(string v)
3737
[OnDeserializing]
3838
public void Deserialize()
3939
{
40-
var v = "invalid";
40+
var v = $"invalid";
4141
f = v; // BAD: False negative
4242

4343
if (v == "valid")
@@ -63,7 +63,7 @@ public Test3(string v)
6363
[OnDeserializing]
6464
public void Deserialize()
6565
{
66-
var v = "invalid";
66+
var v = $"invalid";
6767
f = v; // GOOD: False negative
6868
Assign(v);
6969
}
@@ -95,7 +95,7 @@ public Test4(string v)
9595
[OnDeserializing]
9696
public void Deserialize()
9797
{
98-
var v = "invalid";
98+
var v = $"invalid";
9999
if (v == "valid")
100100
Assign(v);
101101
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
| RuntimeChecksBypass.cs:20:13:20:21 | "invalid" | This write to $@ may be circumventing a $@. | RuntimeChecksBypass.cs:7:19:7:19 | f | f | RuntimeChecksBypass.cs:11:9:14:9 | if (...) ... | check |
1+
| RuntimeChecksBypass.cs:20:13:20:22 | $"..." | This write to $@ may be circumventing a $@. | RuntimeChecksBypass.cs:7:19:7:19 | f | f | RuntimeChecksBypass.cs:11:9:14:9 | if (...) ... | check |
22
| RuntimeChecksBypass.cs:124:15:124:34 | call to method GetInt32 | This write to $@ may be circumventing a $@. | RuntimeChecksBypass.cs:112:16:112:18 | Age | Age | RuntimeChecksBypass.cs:116:9:117:53 | if (...) ... | check |
33
| RuntimeChecksBypass.cs:168:15:168:17 | access to local variable age | This write to $@ may be circumventing a $@. | RuntimeChecksBypass.cs:153:16:153:18 | Age | Age | RuntimeChecksBypass.cs:157:9:158:53 | if (...) ... | check |
44
| RuntimeChecksBypassBad.cs:19:15:19:34 | call to method GetInt32 | This write to $@ may be circumventing a $@. | RuntimeChecksBypassBad.cs:7:16:7:18 | Age | Age | RuntimeChecksBypassBad.cs:11:9:12:53 | if (...) ... | check |

0 commit comments

Comments
 (0)