hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #16186 from michaelnebel/csharp/suppressnullablefix

Browse Source 
C#: Fix issue with suppress nullable warning directly on a method call.
This commit is contained in:
Michael Nebel
2024-04-15 13:11:03 +02:00
committed by GitHub
parent 611cf231a7 cbb5d433b1
commit d5073df24c
11 changed files with 74 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
csharp/extractor/Semmle.Extraction.CSharp/Entities/CallTypeExtensions.cs
View File

@@ -9,7 +9,7 @@ namespace Semmle.Extraction.CSharp.Entities
/// </summary>
public static ExprKind AdjustKind(this Expression.CallType ct, ExprKind k)
{
if (k == ExprKind.ADDRESS_OF)
if (k == ExprKind.ADDRESS_OF || k == ExprKind.SUPPRESS_NULLABLE_WARNING)
{
return k;
}

2
csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/PostfixUnary.cs
View File

@@ -21,11 +21,11 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
protected override void PopulateExpression(TextWriter trapFile)
{
Create(Context, operand, this, 0);
OperatorCall(trapFile, Syntax);
if ((operatorKind == ExprKind.POST_INCR || operatorKind == ExprKind.POST_DECR) &&
Kind == ExprKind.OPERATOR_INVOCATION)
{
OperatorCall(trapFile, Syntax);
trapFile.mutator_invocation_mode(this, 2);
}
}

4
csharp/ql/lib/change-notes/2024-04-12-suppress-nullable-warning.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Extracting suppress nullable warning expressions did not work when applied directly to a method call (like `System.Console.Readline()!`). This has been fixed.

1
csharp/ql/test/library-tests/expressions/ConstructorInitializers.expected
View File

@@ -22,6 +22,7 @@
| file://:0:0:0:0 | Rectangle | expressions.cs:351:18:351:26 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | Rectangle2 | expressions.cs:361:18:361:27 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | ReducedClass | ReducedExpression.cs:2:7:2:18 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | SuppressNullableWarning | expressions.cs:522:11:522:33 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | TestConversionOperator | expressions.cs:330:11:330:32 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | TestCreations | expressions.cs:383:18:383:30 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | TestUnaryOperator | expressions.cs:292:11:292:27 | call to constructor Object | file://:0:0:0:0 | Object |

23
csharp/ql/test/library-tests/expressions/PrintAst.expected
View File

@@ -2406,3 +2406,26 @@ expressions.cs:
# 520| -1: [TypeMention] object
# 520| 3: [ConstructorInitializer] call to constructor ClassC1
# 520| 0: [ParameterAccess] access to parameter oc2
# 522| 24: [Class] SuppressNullableWarning
# 525| 5: [Method] Api
# 525| -1: [TypeMention] object
# 525| 4: [ObjectCreation] object creation of type Object
# 525| 0: [TypeMention] object
# 527| 6: [Method] Test
# 527| -1: [TypeMention] Void
#-----| 2: (Parameters)
# 527| 0: [Parameter] arg0
# 527| -1: [TypeMention] object
# 528| 4: [BlockStmt] {...}
# 529| 0: [LocalVariableDeclStmt] ... ...;
# 529| 0: [LocalVariableDeclAndInitExpr] Object x = ...
# 529| -1: [TypeMention] object
# 529| 0: [LocalVariableAccess] access to local variable x
# 529| 1: [SuppressNullableWarningExpr] ...!
# 529| 0: [ParameterAccess] access to parameter arg0
# 530| 1: [LocalVariableDeclStmt] ... ...;
# 530| 0: [LocalVariableDeclAndInitExpr] Object y = ...
# 530| -1: [TypeMention] object
# 530| 0: [LocalVariableAccess] access to local variable y
# 530| 1: [SuppressNullableWarningExpr] ...!
# 530| 0: [MethodCall] call to method Api

1
csharp/ql/test/library-tests/expressions/QualifiableExpr.expected
View File

@@ -70,3 +70,4 @@
| expressions.cs:483:17:483:26 | access to field value | expressions.cs:483:17:483:20 | this access |
| expressions.cs:488:32:488:39 | access to field value | expressions.cs:488:32:488:33 | access to parameter c1 |
| expressions.cs:488:43:488:50 | access to field value | expressions.cs:488:43:488:44 | access to parameter c2 |
| expressions.cs:530:21:530:25 | call to method Api | expressions.cs:530:21:530:25 | this access |

2
csharp/ql/test/library-tests/expressions/SuppressNullableWarning.expected Normal file
View File

@@ -0,0 +1,2 @@
| expressions.cs:529:21:529:25 | ...! |
| expressions.cs:530:21:530:26 | ...! |

3
csharp/ql/test/library-tests/expressions/SuppressNullableWarning.ql Normal file
View File

@@ -0,0 +1,3 @@
import csharp
select any(SuppressNullableWarningExpr e)

12
csharp/ql/test/library-tests/expressions/expressions.cs
View File

@@ -518,4 +518,16 @@ namespace Expressions
class ClassC1(object oc1) { }
class ClassC2(object oc2) : ClassC1(oc2) { }
class SuppressNullableWarning
{
public object? Api() => new object();
public void Test(object? arg0)
{
var x = arg0!;
var y = Api()!;
}
}
}

11
csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.cs
View File

@@ -95,6 +95,17 @@ namespace Test
var result = new DataSet();
adapter.Fill(result);
}
// BAD: Input from the command line. (also implicitly check flow via suppress nullable warning `!`)
using (var connection = new SqlConnection(connectionString))
{
var queryString = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
+ Console.ReadLine()! + "' ORDER BY PRICE";
var cmd = new SqlCommand(queryString);
var adapter = new SqlDataAdapter(cmd);
var result = new DataSet();
adapter.Fill(result);
}
}
System.Windows.Forms.TextBox box1;

15
csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.expected
View File

@@ -27,6 +27,12 @@ edges
| SqlInjection.cs:93:21:93:23 | access to local variable cmd : SqlCommand | SqlInjection.cs:94:50:94:52 | access to local variable cmd | provenance | Sink:MaD:950 |
| SqlInjection.cs:93:27:93:53 | object creation of type SqlCommand : SqlCommand | SqlInjection.cs:93:21:93:23 | access to local variable cmd : SqlCommand | provenance | |
| SqlInjection.cs:93:42:93:52 | access to local variable queryString : String | SqlInjection.cs:93:27:93:53 | object creation of type SqlCommand : SqlCommand | provenance | MaD:953 |
| SqlInjection.cs:102:21:102:31 | access to local variable queryString : String | SqlInjection.cs:104:42:104:52 | access to local variable queryString | provenance | Sink:MaD:947 |
| SqlInjection.cs:102:21:102:31 | access to local variable queryString : String | SqlInjection.cs:104:42:104:52 | access to local variable queryString : String | provenance | |
| SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | SqlInjection.cs:102:21:102:31 | access to local variable queryString : String | provenance | Src:MaD:2250 |
| SqlInjection.cs:104:21:104:23 | access to local variable cmd : SqlCommand | SqlInjection.cs:105:50:105:52 | access to local variable cmd | provenance | Sink:MaD:950 |
| SqlInjection.cs:104:27:104:53 | object creation of type SqlCommand : SqlCommand | SqlInjection.cs:104:21:104:23 | access to local variable cmd : SqlCommand | provenance | |
| SqlInjection.cs:104:42:104:52 | access to local variable queryString : String | SqlInjection.cs:104:27:104:53 | object creation of type SqlCommand : SqlCommand | provenance | MaD:953 |
| SqlInjectionDapper.cs:20:21:20:25 | access to local variable query : String | SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | provenance | Sink:MaD:27 |
| SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | SqlInjectionDapper.cs:20:21:20:25 | access to local variable query : String | provenance | |
| SqlInjectionDapper.cs:29:21:29:25 | access to local variable query : String | SqlInjectionDapper.cs:30:66:30:70 | access to local variable query | provenance | Sink:MaD:37 |
@@ -97,6 +103,13 @@ nodes
| SqlInjection.cs:93:42:93:52 | access to local variable queryString | semmle.label | access to local variable queryString |
| SqlInjection.cs:93:42:93:52 | access to local variable queryString : String | semmle.label | access to local variable queryString : String |
| SqlInjection.cs:94:50:94:52 | access to local variable cmd | semmle.label | access to local variable cmd |
| SqlInjection.cs:102:21:102:31 | access to local variable queryString : String | semmle.label | access to local variable queryString : String |
| SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | semmle.label | call to method ReadLine : String |
| SqlInjection.cs:104:21:104:23 | access to local variable cmd : SqlCommand | semmle.label | access to local variable cmd : SqlCommand |
| SqlInjection.cs:104:27:104:53 | object creation of type SqlCommand : SqlCommand | semmle.label | object creation of type SqlCommand : SqlCommand |
| SqlInjection.cs:104:42:104:52 | access to local variable queryString | semmle.label | access to local variable queryString |
| SqlInjection.cs:104:42:104:52 | access to local variable queryString : String | semmle.label | access to local variable queryString : String |
| SqlInjection.cs:105:50:105:52 | access to local variable cmd | semmle.label | access to local variable cmd |
| SqlInjectionDapper.cs:20:21:20:25 | access to local variable query : String | semmle.label | access to local variable query : String |
| SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | semmle.label | access to property Text : String |
| SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | semmle.label | access to local variable query |
@@ -154,6 +167,8 @@ subpaths
| SqlInjection.cs:83:50:83:55 | access to local variable query1 | SqlInjection.cs:82:21:82:29 | access to property Text : String | SqlInjection.cs:83:50:83:55 | access to local variable query1 | This query depends on $@. | SqlInjection.cs:82:21:82:29 | access to property Text : String | this TextBox text |
| SqlInjection.cs:93:42:93:52 | access to local variable queryString | SqlInjection.cs:92:21:92:29 | access to property Text : String | SqlInjection.cs:93:42:93:52 | access to local variable queryString | This query depends on $@. | SqlInjection.cs:92:21:92:29 | access to property Text : String | this TextBox text |
| SqlInjection.cs:94:50:94:52 | access to local variable cmd | SqlInjection.cs:92:21:92:29 | access to property Text : String | SqlInjection.cs:94:50:94:52 | access to local variable cmd | This query depends on $@. | SqlInjection.cs:92:21:92:29 | access to property Text : String | this TextBox text |
| SqlInjection.cs:104:42:104:52 | access to local variable queryString | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | SqlInjection.cs:104:42:104:52 | access to local variable queryString | This query depends on $@. | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | this external |
| SqlInjection.cs:105:50:105:52 | access to local variable cmd | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | SqlInjection.cs:105:50:105:52 | access to local variable cmd | This query depends on $@. | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | this external |
| SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | This query depends on $@. | SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | this TextBox text |
| SqlInjectionDapper.cs:30:66:30:70 | access to local variable query | SqlInjectionDapper.cs:29:86:29:94 | access to property Text : String | SqlInjectionDapper.cs:30:66:30:70 | access to local variable query | This query depends on $@. | SqlInjectionDapper.cs:29:86:29:94 | access to property Text : String | this TextBox text |
| SqlInjectionDapper.cs:39:63:39:67 | access to local variable query | SqlInjectionDapper.cs:38:86:38:94 | access to property Text : String | SqlInjectionDapper.cs:39:63:39:67 | access to local variable query | This query depends on $@. | SqlInjectionDapper.cs:38:86:38:94 | access to property Text : String | this TextBox text |