Commit Graph

688 Commits

Author SHA1 Message Date
Calum Grant
20752c80c9 C#: Address review comments 2019-06-04 18:10:49 +01:00
Tom Hvitved
6b99e1a1bf C#: Teach cs/useless-upcast about disambiguating constructor calls 2019-05-29 15:16:08 +02:00
Tom Hvitved
d8482083b9 C#: Add tests for disambiguated constructor calls 2019-05-29 15:14:30 +02:00
Tom Hvitved
a1e58cedac C#: Refactor recursive patterns implementation
- Extract names of properties in a propery match, using the `exprorstmt_name` relation.
- Simplify extraction of properties by not distinguishing between top-level patterns
  and nested patterns.
- Introduce `PatternExpr` to capture patterns in `is` expressions, `case` statements,
  and `switch` expression arms.
- Generalize `IsTypeExpr`, `IsPatternExpr`, `IsRecursivePatternExpr`, and `IsConstantExpr`
  to just `IsExpr` with a member predicate `PatternExpr getPattern()`.
- Generalize `TypeCase`, `RecursivePatternCase`, and `ConstCase` to just `CaseStmt` with
  a member predicate `PatternExpr getPattern()`.
- Introduce classes `Switch` and `Case` as base classes of switch statements/expressions
  and case statements/switch expression arms, respectively.
- Simplify CFG logic using the generalized classes.
- Generalize guards library to cover `switch` expressions tests.
- Generalize data flow library to cover `switch` expression assignments.
2019-05-24 13:49:05 +01:00
Denis Levin
eacded27a9 Japanese Era and Leap Year checks (Likely Bugs) 2019-05-20 15:54:57 -07:00
Tom Hvitved
949b3601d0 C#: Address review comments 2019-05-15 14:10:42 +02:00
Tom Hvitved
c6a471e4b6 C#: Adopt shared data flow implementation
- General refactoring to fit with the shared data flow implementation.
- Move CFG splitting logic into `ControlFlowReachability.qll`.
- Replace `isAdditionalFlowStepIntoCall()` with `TaintedParameterNode`.
- Redefine `ReturnNode` to be the actual values that are returned, which should
  yield better path information.
- No longer consider overrides in CIL calls.
2019-05-06 14:54:11 +02:00
calum
b628060ddd C#: Address review comments. 2019-04-16 17:38:55 +01:00
calum
b11dce7b8a C#: Fix FP in expressions of the form (a?b:null)??d, where the null has a single successor, but the d is a join node. 2019-04-16 16:44:51 +01:00
calum
3e563f7a11 C#: Add (failing) unit test for cs/constant-condition 2019-04-16 16:44:50 +01:00
calum
42b2f09315 C#: Tidy up query, remove false-positives and add some more test cases. 2019-04-03 12:17:01 +01:00
Calum Grant
01aa4ecf2f Merge pull request #1075 from hvitved/csharp/get-location-to-string
C#: Simplify dispatch hierarchy for `getLocation()` and `toString()`
2019-03-26 12:56:29 +00:00
Tom Hvitved
1d05bccd87 Merge pull request #952 from calumgrant/cs/non-null-functions
C#: Better call analysis using CIL
2019-03-23 10:47:22 +01:00
Tom Hvitved
6c182564e7 C#: Adjustments to CIL/nullness analyses
- Cache predicates in the same stage using a cached module.
- Introduce `DefUse::defUseVariableUpdate()` and use in `CallableReturns.qll`.
  The updated file `csharp/ql/test/library-tests/cil/dataflow/Nullness.expected`
  demonstrates why this is needed.
- Utilize CIL analysis in `Guards::nonNullValue()`.
- Analyze SSA definitions in `AlwaysNullExpr`, similar to `NonNullExpr`.
2019-03-22 15:11:31 +01:00
Calum Grant
5a3cf2c5bb Merge pull request #1054 from raulgarciamsft/users/raulga/ICryptoTransformLambda
2n part of ICryptoTransform.
2019-03-15 12:55:09 +00:00
Raul Garcia
2521848322 Merging the scenarios. 2019-03-14 10:57:22 -07:00
Raul Garcia
d4825afc79 Updating expected results (I forgot to update them on my previous push) 2019-03-12 10:14:07 -07:00
Tom Hvitved
6d5330b01e C#: Replace getLocation() with hasLocationInfo() in Attributable 2019-03-12 09:09:43 +01:00
Raul Garcia
4ad9163fa8 I fixed the typos, and added support for Parallel.Invoke.
Please let em know if this pattern works, and I can add other mechanisms to start new threads with a shared object.
Please also let me know what other mechanisms would you like me to add, I would like to focus on the most commonly used ones first. Thanks
2019-03-11 16:25:49 -07:00
Tom Hvitved
b48576d7b9 C#: Address review comments 2019-03-10 15:45:31 +01:00
Tom Hvitved
8959d528a1 Merge remote-tracking branch 'upstream/rc/1.20' into csharp/dataflow/performance 2019-03-10 15:07:18 +01:00
Tom Hvitved
e6f7632d4c C#: Introduce data flow return nodes
Before this change,

```
flowOutOfCallableStep(CallNode call, ReturnNode ret, OutNode out, CallContext cc)
```

would compute all combinations of call sites `call` and returned expressions `ret`
up front.

Now, we instead introduce explicit return nodes, so each callable has exactly
one return node (as well as one for each `out`/`ref` parameter). There is then
local flow from a returned expression to the relevant return node, and
`flowOutOfCallableStep()` computes combinations of call sites and return nodes.

Not only does this result in better performance, it also makes `flowOutOfCallableStep()`
symmetric to `flowIntoCallableStep()`, where each argument is mapped to a parameter,
and not to all reads of that parameter.
2019-03-07 12:16:06 +01:00
Raul Garcia
2e0c337a94 2n part of ICryptoTransform.
Detecting potential unsafe usage (object shared across multiple threads) on variables captured by Lambda
2019-03-06 17:12:33 -08:00
Tom Hvitved
440809623b C#: Fix whitespaces 2019-03-06 08:15:46 +01:00
Max Schaefer
7f5e2630a1 Merge pull request #1032 from xiemaisi/master-for-merge
Merge master into rc/1.20
2019-03-04 21:23:51 +00:00
Tom Hvitved
c70a0a646d C#: Add test for getLabel() 2019-03-04 13:21:10 +01:00
Tom Hvitved
51e5a301cd Merge pull request #956 from raulgarciamsft/users/raulga/ICryptoTransform
Detect usage of ICryptoTransform that would be thread-unsafe
2019-03-01 11:49:27 +01:00
Calum Grant
c945b7793c Merge pull request #944 from hvitved/csharp/cfg/accessor-call
C#: Improve CFG for assignments
2019-02-28 09:34:56 +00:00
Raul Garcia
9bb7816a3c Making changes based on feedback. 2019-02-22 10:10:20 -08:00
calum
15341965e0 C#: Update cs/use-of-vulnerable-package to detect CVE-2019-0657 2019-02-21 11:48:48 +00:00
Raul Garcia
7d197692ac Adding a new rule for detecting usage of static objects that implement ICryptoTransform that would be thread-unsafe, and potentially result in incorrect cryptographic results. 2019-02-20 17:07:04 -08:00
Tom Hvitved
5ce9b25ec9 C#: Improve CFG for assignments
Write accesses in assignments, such as the access to `x` in `x = 0` are not
evaluated, so they should not have entries in the control flow graph. However,
qualifiers (and indexer arguments) should still be evaluated, for example in

```
x.Foo.Bar = 0;
```

the CFG should be `x --> x.Foo --> 0 --> x.Foo.Bar = 0` (as opposed to
`x --> x.Foo --> x.Foo.Bar --> 0 --> x.Foo.Bar = 0`, prior to this change).

A special case is assignments via acessors (properties, indexers, and event
adders), where we do want to include the access in the control flow graph,
as it represents the accessor call:

```
x.Prop = 0;
```

But instead of `x --> x.set_Prop --> 0 --> x.Prop = 0` the CFG should be
`x --> 0 --> x.set_Prop --> x.Prop = 0`, as the setter is called *after* the
assigned value has been evaluated.

An even more special case is tuple assignments via accessors:

```
(x.Prop1, y.Prop2) = (0, 1);
```

Here the CFG should be
`x --> y --> 0 --> 1 --> x.set_Prop1 --> y.set_Prop2 --> (x.Prop1, y.Prop2) = (0, 1)`.
2019-02-16 19:19:24 +01:00
calum
d18bbf6a73 C#: Make query only apply to reftypes, since I believe valuetypes are safe or cannot be fixed trivially using the volatile keyword. 2019-02-08 15:18:29 +00:00
calum
7addd41e38 C#: Fixes to double-checked lock. 2019-02-08 14:57:57 +00:00
Calum Grant
73d56e1bdb Merge pull request #881 from hvitved/csharp/remove-get-url
C#: Remove `getUrl()` predicate
2019-02-07 10:47:56 +00:00
Tom Hvitved
37c55750f7 Merge pull request #873 from calumgrant/cs/format-getresource-strings
C#: Fix FP in cs/format-argument-unused
2019-02-05 17:12:04 +01:00
Tom Hvitved
0211837e24 C#: Remove getUrl() predicate 2019-02-05 11:07:13 +01:00
Tom Hvitved
b4b6fdd12b C#: Revert recent change to AccessorCall
The recent change to `AccessorCall` on dd99525566 resulted
in some bad join-orders, so I have (partly) reverted them. This means that the issues
orignally addressed by that change are now reintroduced, and I plan to instead apply a
fix to the CFG, which--unlike the original fix--should be able to handle multi-property-tuple
assignments.
2019-02-04 15:14:18 +01:00
calum
7d17724cae C#: The empty string is not considered a format string for this query. 2019-02-04 12:53:12 +00:00
calum
eb0036172e C#: Add test for false-positive. 2019-02-04 12:30:43 +00:00
calum
d63df71a8a C#: Fix merge conflict. 2019-01-29 18:15:33 +00:00
calum
423513169f C#: Address review comments. Introduce Member::isEffectivelyPublic() because isEffectivelyPrivate and isEffectivelyInternal are almost always used together. 2019-01-29 18:05:29 +00:00
calum
931b6b4ee5 C#: Exclude interfaces and abstract classes from cs/call-to-object-tostring 2019-01-29 18:04:26 +00:00
Calum Grant
eef1abfa69 Merge pull request #743 from hvitved/csharp/dataflow-splitting
C#: Teach data flow library about CFG splitting
2019-01-28 16:31:24 +00:00
Calum Grant
c6d0600e76 Merge pull request #798 from hvitved/csharp/accessor-calls
C#: Redefine `AccessorCall`
2019-01-24 10:21:32 +00:00
calum
420c943cce C#: Fix FP in cs/call-to-object-tostring 2019-01-23 16:14:25 +00:00
calum
c9ffb38e4b C#: Add sources and sinks in Winforms. Update some queries with new sources and sinks. 2019-01-18 15:42:44 +00:00
Tom Hvitved
dd99525566 C#: Redefine AccessorCall
The syntactic node assiociated with accessor calls was previously always the
underlying member access. For example, in

```
x.Prop = y.Prop;
```

the implicit call to `x.set_Prop()` was at the syntactic node `x.Prop`, while the
implicit call to `y.get_Prop()` was at the syntactic node `y.Prop`.

However, this breaks the invariant that arguments to calls dominate the call itself,
as the argument `y.Prop` for the implicit `value` parameter in `x.set_Prop()` will
be evaluated after the call (the left-hand side in an assignment is evaluated before
the right-hand side).

The solution is to redefine the access call to `x.set_Prop()` to point to the whole
assignment `x.Prop = y.Prop`, instead of the access `x.Prop`. For reads, we still want
to associate the accessor call with the member access.

A corner case arises when multiple setters are called in a tuple assignment:

```
(x.Prop1, x.Prop2) = (0, 1)
```

In this case, we cannot associate the assignment with both `x.set_Prop1()` and
`x.set_Prop2()`, so we instead revert to using the underlying member accesses as
before.
2019-01-18 13:56:23 +01:00
Tom Hvitved
2caf724826 C#: Add more tests 2019-01-18 12:07:22 +01:00
Tom Hvitved
b2f99dbbc7 C#: Teach data flow library about CFG splitting
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.
2019-01-16 10:39:27 +01:00