mirror of
https://github.com/github/codeql.git
synced 2025-12-20 10:46:30 +01:00
b2f99dbbc7
Data flow nodes for expressions do not take CFG splitting into account. Example:
```
if (b)
x = tainted;
x = x.ToLower();
if (!b)
Use(x);
```
Flow is incorrectly reported from `tainted` to `x` in `Use(x)`, because the step
from `tainted` to `x.ToLower()` throws away the information that `b = true`.
The solution is to remember the splitting in data flow expression nodes, that is,
to represent the exact control flow node instead of just the expression. With that
we get flow from `tainted` to `[b = true] x.ToLower()`, but not from `tainted` to
`[b = false] x.ToLower()`.
The data flow API remains unchanged, but in order for analyses to fully benefit from
CFG splitting, sanitizers in particular should be CFG-based instead of expression-based:
```
if (b)
x = tainted;
if (IsInvalid(x))
return;
Use(x);
```
If the call to `IsInvalid()` is a sanitizer, then defining an expression node to be
a sanitizer using `GuardedExpr` will be too conservative (`x` in `Use(x)` is in fact
not guarded). However, `[b = true] x` in `[b = true] Use(x)` is guarded, and to help
defining guard-based sanitizers, the class `GuardedDataFlowNode` has been introduced.
74 lines
1.8 KiB
C#
74 lines
1.8 KiB
C#
//semmle-extractor-options: ${testdir}/../../../resources/stubs/System.Web.cs /r:System.Collections.Specialized.dll
|
|
|
|
using System;
|
|
using System.Web;
|
|
|
|
public class StackTraceHandler : IHttpHandler
|
|
{
|
|
bool b;
|
|
public void ProcessRequest(HttpContext ctx)
|
|
{
|
|
try
|
|
{
|
|
doSomeWork();
|
|
}
|
|
catch (Exception ex)
|
|
{
|
|
// BAD: printing a stack trace back to the response
|
|
ctx.Response.Write(ex.ToString());
|
|
// BAD: implicitly printing a stack trace back to the response
|
|
ctx.Response.Write(ex);
|
|
// BAD: writing StackTrace property to response
|
|
ctx.Response.Write(ex.StackTrace);
|
|
// GOOD: writing Message property to response
|
|
ctx.Response.Write(ex.Message);
|
|
return;
|
|
}
|
|
|
|
try
|
|
{
|
|
doSomeWork();
|
|
}
|
|
catch (Exception ex)
|
|
{
|
|
// GOOD: log the stack trace, and send back a non-revealing response
|
|
log("Exception occurred", ex);
|
|
ctx.Response.Write("Exception occurred");
|
|
return;
|
|
}
|
|
|
|
// BAD: printing a stack trace back to the response for a custom exception
|
|
ctx.Response.Write(new MyException().ToString());
|
|
}
|
|
|
|
class MyException : Exception
|
|
{
|
|
private Exception nested;
|
|
string ToString()
|
|
{
|
|
// IGNORED - the outer ToString() should be reported, not this nested call
|
|
return nested.ToString();
|
|
}
|
|
}
|
|
|
|
// Method that may throw an exception
|
|
public void doSomeWork()
|
|
{
|
|
if (b)
|
|
throw new Exception();
|
|
}
|
|
|
|
public void log(string s, Exception e)
|
|
{
|
|
// logging stub
|
|
}
|
|
|
|
public bool IsReusable
|
|
{
|
|
get
|
|
{
|
|
return true;
|
|
}
|
|
}
|
|
}
|