Skip to content

Commit

Permalink
NtstatusExplicitCast2: CodeQL port of C29715 (#150)
Browse files Browse the repository at this point in the history 
* port of C29715

* add other bool types
  • Loading branch information
@jacob-ronstadt
jacob-ronstadt committed Aug 23, 2024
1 parent 735f49f commit d71fb93
Show file tree
Hide file tree
Showing 5 changed files with 417 additions and 0 deletions.
55 changes: 55 additions & 0 deletions src/drivers/general/queries/NtstatusExplicitCast2/NtstatusExplicitCast2.qhelp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
Cast between semantically different integer types. This warning indicates that a Boolean is being cast to NTSTATUS. This is likely to give undesirable results. For example, the typical failure value for functions that return a Boolean (FALSE) is a success status when tested as an NTSTATUS.
</p>
</overview>
<recommendation>
<p>
Typically, a function that returns Boolean returns either 1 (for TRUE) or 0 (for FALSE). Both these values are treated as success codes by the NT_SUCCESS macro. Thus, the failure case will never be detected.
</p>

</recommendation>
<example>
<p>
Bad cast from Boolean to NTSTATUS
</p>
<sample language="c"> <![CDATA[
if (NT_SUCCESS(SomeFunction()))
{
return 0;
}
else
{
return -1;
}
}]]>
</sample>
<p>
Correct use of Boolean
</p>
<sample language="c"> <![CDATA[
if (SomeFunction() == TRUE)
{
return 0;
}
else
{
return -1;
}
}]]>
</sample>
</example>
<semmleNotes>
<p>
</p>
</semmleNotes>
<references>
<li>
<a href="https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/28715-boolean-cast-between-semantically-different-integer-types">
Warning C28715
</a>
</li>
</references>
</qhelp>
30 changes: 30 additions & 0 deletions src/drivers/general/queries/NtstatusExplicitCast2/NtstatusExplicitCast2.ql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
/**
* @id cpp/drivers/ntstatus-explicit-cast2
* @kind problem
* @name Ntstatus Explicit Cast 2
* @description Cast between semantically different integer types (Boolean to NTSTATUS).
* @platform Desktop
* @feature.area Multiple
* @impact Insecure Coding Practice
* @repro.text This warning indicates that a Boolean is being cast to NTSTATUS. This is likely to give undesirable results. For example, the typical failure value for functions that return a Boolean (FALSE) is a success status when tested as an NTSTATUS.
* @opaqueid CQLD-C28715
* @problem.severity warning
* @precision medium
* @tags correctness
* @scope domainspecific
* @query-version v1
*/

import cpp

from Conversion c
where
(
c.getType().toString().toLowerCase().matches("boolean") or
c.getType().toString().toLowerCase().matches("bool") or
c.getType().toString().matches("VARIANT_BOOL")
) and
c.getConversion().getType().toString().matches("NTSTATUS")
select c, "Cast between semantically different integer types: Boolean to NTSTATUS"
273 changes: 273 additions & 0 deletions src/drivers/general/queries/NtstatusExplicitCast2/NtstatusExplicitCast2.sarif
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,273 @@
{
"$schema": "https://json.schemastore.org/sarif-2.1.0.json",
"version": "2.1.0",
"runs": [
{
"tool": {
"driver": {
"name": "CodeQL",
"organization": "GitHub",
"semanticVersion": "2.17.6",
"notifications": [
{
"id": "cpp/baseline/expected-extracted-files",
"name": "cpp/baseline/expected-extracted-files",
"shortDescription": {
"text": "Expected extracted files"
},
"fullDescription": {
"text": "Files appearing in the source archive that are expected to be extracted."
},
"defaultConfiguration": {
"enabled": true
},
"properties": {
"tags": [
"expected-extracted-files",
"telemetry"
]
}
},
{
"id": "cpp/extractor/summary",
"name": "cpp/extractor/summary",
"shortDescription": {
"text": "C++ extractor telemetry"
},
"fullDescription": {
"text": "C++ extractor telemetry"
},
"defaultConfiguration": {
"enabled": true
}
}
],
"rules": [
{
"id": "cpp/drivers/ntstatus-explicit-cast2",
"name": "cpp/drivers/ntstatus-explicit-cast2",
"shortDescription": {
"text": "Ntstatus Explicit Cast 2"
},
"fullDescription": {
"text": "Cast between semantically different integer types (Boolean to NTSTATUS)."
},
"defaultConfiguration": {
"enabled": true,
"level": "warning"
},
"properties": {
"tags": [
"correctness"
],
"description": "Cast between semantically different integer types (Boolean to NTSTATUS).",
"feature.area": "Multiple",
"id": "cpp/drivers/ntstatus-explicit-cast2",
"impact": "Insecure Coding Practice",
"kind": "problem",
"name": "Ntstatus Explicit Cast 2",
"opaqueid": "CQLD-C28715",
"platform": "Desktop",
"precision": "medium",
"problem.severity": "warning",
"query-version": "v1",
"repro.text": "This warning indicates that a Boolean is being cast to NTSTATUS. This is likely to give undesirable results. For example, the typical failure value for functions that return a Boolean (FALSE) is a success status when tested as an NTSTATUS.",
"scope": "domainspecific"
}
}
]
},
"extensions": [
{
"name": "microsoft/windows-drivers",
"semanticVersion": "1.1.0+ce7d70c32c8e0908d7c329389aa84ac3a89e7feb",
"locations": [
{
"uri": "file:///C:/codeql-home/WDDST/src/",
"description": {
"text": "The QL pack root directory."
}
},
{
"uri": "file:///C:/codeql-home/WDDST/src/qlpack.yml",
"description": {
"text": "The QL pack definition file."
}
}
]
}
]
},
"invocations": [
{
"toolExecutionNotifications": [
{
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "driver/driver_snippet.c",
"uriBaseId": "%SRCROOT%",
"index": 0
}
}
}
],
"message": {
"text": ""
},
"level": "none",
"descriptor": {
"id": "cpp/baseline/expected-extracted-files",
"index": 0
},
"properties": {
"formattedMessage": {
"text": ""
}
}
},
{
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "driver/fail_driver1.h",
"uriBaseId": "%SRCROOT%",
"index": 1
}
}
}
],
"message": {
"text": ""
},
"level": "none",
"descriptor": {
"id": "cpp/baseline/expected-extracted-files",
"index": 0
},
"properties": {
"formattedMessage": {
"text": ""
}
}
},
{
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "driver/fail_driver1.c",
"uriBaseId": "%SRCROOT%",
"index": 2
}
}
}
],
"message": {
"text": ""
},
"level": "none",
"descriptor": {
"id": "cpp/baseline/expected-extracted-files",
"index": 0
},
"properties": {
"formattedMessage": {
"text": ""
}
}
},
{
"message": {
"text": "Internal telemetry for the C++ extractor.\n\nNo action needed.",
"markdown": "Internal telemetry for the C++ extractor.\n\nNo action needed."
},
"level": "note",
"timeUtc": "2024-08-21T06:06:26.075+00:00",
"descriptor": {
"id": "cpp/extractor/summary",
"index": 1
},
"properties": {
"attributes": {
"cache-hits": 0,
"cache-misses": 1,
"extractor-failures": 1,
"extractor-successes": 0,
"trap-caching": "disabled"
},
"visibility": {
"statusPage": false,
"telemetry": true
}
}
}
],
"executionSuccessful": true
}
],
"artifacts": [
{
"location": {
"uri": "driver/driver_snippet.c",
"uriBaseId": "%SRCROOT%",
"index": 0
}
},
{
"location": {
"uri": "driver/fail_driver1.h",
"uriBaseId": "%SRCROOT%",
"index": 1
}
},
{
"location": {
"uri": "driver/fail_driver1.c",
"uriBaseId": "%SRCROOT%",
"index": 2
}
}
],
"results": [
{
"ruleId": "cpp/drivers/ntstatus-explicit-cast2",
"ruleIndex": 0,
"rule": {
"id": "cpp/drivers/ntstatus-explicit-cast2",
"index": 0
},
"message": {
"text": "Cast between semantically different integer types: Boolean to NTSTATUS"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "driver/driver_snippet.c",
"uriBaseId": "%SRCROOT%",
"index": 0
},
"region": {
"startLine": 30,
"startColumn": 9,
"endColumn": 35
}
}
}
],
"partialFingerprints": {
"primaryLocationLineHash": "bbf24f16ac513f29:1",
"primaryLocationStartColumnFingerprint": "4"
}
}
],
"columnKind": "utf16CodeUnits",
"properties": {
"semmle.formatSpecifier": "sarifv2.1.0"
}
}
]
}
Loading

0 comments on commit d71fb93

Please sign in to comment.