Skip to content

Commit

Permalink
port of C28716
Browse files Browse the repository at this point in the history
  • Loading branch information
@jacob-ronstadt
jacob-ronstadt committed Aug 20, 2024
1 parent ce7d70c commit 0d7498e
Show file tree
Hide file tree
Showing 5 changed files with 414 additions and 0 deletions.
52 changes: 52 additions & 0 deletions src/drivers/general/queries/NtstatusExplicitCast3/NtstatusExplicitCast3.qhelp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
Compiler-inserted cast between semantically different integral types (Boolean to NTSTATUS). This warning indicates that a Boolean is being used as an NTSTATUS without being explicitly cast.
</p>
</overview>
<recommendation>
<p>
This warning indicates that a Boolean is being used as an NTSTATUS without being explicitly cast. This is likely to give undesirable results. For instance, the typical failure value for functions that return a Boolean (false) indicates a success status when tested as an NTSTATUS.
</p>
</recommendation>
<example>
<p>
Example of SomeMemAllocFunction that has a Boolean return value but is being returned in a function with NTSTATUS return type.
</p>
<sample language="c"> <![CDATA[
extern bool SomeMemAllocFunction(void **);
NTSTATUS test_bad()
{
void *MyPtr;
return SomeMemAllocFunction(&MyPtr);
}
}]]>
</sample>
<p>
This example avoids the warning
</p>
<sample language="c"> <![CDATA[
extern bool SomeMemAllocFunction(void **);
if (SomeMemAllocFunction(&MyPtr) == true) {
return STATUS_SUCCESS;
} else {
return STATUS_NO_MEMORY;
}
}]]>
</sample>
</example>
<semmleNotes>
<p>
</p>
</semmleNotes>
<references>
<li>
<a href="https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/28716-compiler-inserted-cast-between-semantically-different-integral">
Warning C28716
</a>
</li>
</references>
</qhelp>
29 changes: 29 additions & 0 deletions src/drivers/general/queries/NtstatusExplicitCast3/NtstatusExplicitCast3.ql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
/**
* @id cpp/drivers/ntstatus-explicit-cast3
* @kind problem
* @name Ntstatus Explicit Cast 3
* @description Compiler-inserted cast between semantically different integral types
* @platform Desktop
* @feature.area Multiple
* @impact Insecure Coding Practice
* @repro.text
* @owner.email: sdat@microsoft.com
* @opaqueid CQLD-C28716
* @problem.severity warning
* @precision medium
* @tags correctness
* @scope domainspecific
* @query-version v1
*/

import cpp

from Conversion c
where
c.isImplicit() and
c.getType().toString().matches("NTSTATUS") and
c.getUnconverted().getType().toString().matches("BOOLEAN")
select c.getUnconverted(),
"Implicit cast between semantically different integer types: Boolean to NTSTATUS"
274 changes: 274 additions & 0 deletions src/drivers/general/queries/NtstatusExplicitCast3/NtstatusExplicitCast3.sarif
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,274 @@
{
"$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-cast3",
"name": "cpp/drivers/ntstatus-explicit-cast3",
"shortDescription": {
"text": "Ntstatus Explicit Cast 3"
},
"fullDescription": {
"text": "Compiler-inserted cast between semantically different integral types"
},
"defaultConfiguration": {
"enabled": true,
"level": "warning"
},
"properties": {
"tags": [
"correctness"
],
"description": "Compiler-inserted cast between semantically different integral types",
"feature.area": "Multiple",
"id": "cpp/drivers/ntstatus-explicit-cast3",
"impact": "Insecure Coding Practice",
"kind": "problem",
"name": "Ntstatus Explicit Cast 3",
"opaqueid": "CQLD-C28716",
"owner.email:": "sdat@microsoft.com",
"platform": "Desktop",
"precision": "medium",
"problem.severity": "warning",
"query-version": "v1",
"repro.text": "",
"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:43:54.207+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-cast3",
"ruleIndex": 0,
"rule": {
"id": "cpp/drivers/ntstatus-explicit-cast3",
"index": 0
},
"message": {
"text": "Implicit cast between semantically different integer types: Boolean to NTSTATUS"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "driver/driver_snippet.c",
"uriBaseId": "%SRCROOT%",
"index": 0
},
"region": {
"startLine": 36,
"startColumn": 12,
"endColumn": 32
}
}
}
],
"partialFingerprints": {
"primaryLocationLineHash": "4f26d0813db0114f:1",
"primaryLocationStartColumnFingerprint": "7"
}
}
],
"columnKind": "utf16CodeUnits",
"properties": {
"semmle.formatSpecifier": "sarifv2.1.0"
}
}
]
}
38 changes: 38 additions & 0 deletions src/drivers/general/queries/NtstatusExplicitCast3/driver_snippet.c
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

// Macros to enable or disable a code section that may or may not conflict with this test.
#define SET_DISPATCH 1

// Template function. Not used for this test.
void top_level_call()
{
}
BOOLEAN SomeMemAllocFunction(void *p)
{
if (p == NULL)
{
return FALSE;
}
return TRUE;
}

NTSTATUS test_good()
{
void *MyPtr;
if (SomeMemAllocFunction(&MyPtr) == TRUE)
{
return STATUS_SUCCESS;
}
else
{
return STATUS_NO_MEMORY;
}
}

NTSTATUS test_bad()
{
void *MyPtr;
return SomeMemAllocFunction(&MyPtr);
}
// TODO add tests for query
Loading

0 comments on commit 0d7498e

Please sign in to comment.