diff --git a/src/drivers/general/queries/IRPStackEntryCopy/IRPStackEntryCopy.qhelp b/src/drivers/general/queries/IRPStackEntryCopy/IRPStackEntryCopy.qhelp new file mode 100644 index 00000000..ebd10110 --- /dev/null +++ b/src/drivers/general/queries/IRPStackEntryCopy/IRPStackEntryCopy.qhelp @@ -0,0 +1,23 @@ + + + +

+ Copying a whole IRP stack entry leaves certain fields initialized that should be cleared or updated +

+
+ +

+ The driver is copying an IRP improperly. Improperly copying an IRP can cause serious problems with a driver, including loss of data and system crashes. If an IRP must be copied and IoCopyCurrentIrpStackLocationToNext does not suffice, then certain members of the IRP should not be copied or should be zeroed after copying. +

+
+ + + + +
  • + + C28114 + +
  • +
    +
    diff --git a/src/drivers/general/queries/IRPStackEntryCopy/IRPStackEntryCopy.ql b/src/drivers/general/queries/IRPStackEntryCopy/IRPStackEntryCopy.ql new file mode 100644 index 00000000..afca8cc9 --- /dev/null +++ b/src/drivers/general/queries/IRPStackEntryCopy/IRPStackEntryCopy.ql @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +/** + * @id cpp/drivers/irp-stack-entry-copy + * @name Irp stack entry copy + * @description C28114: Copying a whole IRP stack entry leaves certain fields initialized that should be cleared or updated. + * @platform Desktop + * @security.severity Medium + * @feature.area Multiple + * @impact Exploitable Design + * @repro.text + * @owner.email sdat@microsoft.com + * @opaqueid CQLD-C28114 + * @kind problem + * @problem.severity warning + * @precision medium + * @tags correctness + * wddst + * @scope domainspecific + * @query-version v1 + */ + +import cpp + +class PIO_STACK_LOCATION extends TypedefType { + PIO_STACK_LOCATION() { + this instanceof TypedefType and + this.getName() = "PIO_STACK_LOCATION" + } +} + +from FunctionCall mem_copy_func, Expr io_stack_param, string size +where + ( + mem_copy_func.getTarget().getName() = "RtlMoveMemory" or + mem_copy_func.getTarget().getName() = "RtlCopyMemory" or + mem_copy_func.getTarget().getName() = "RtlCopyBytes" or + mem_copy_func.getTarget().getName() = "RtlCopyMemory32" or + mem_copy_func.getTarget().getName() = "memmove" or + mem_copy_func.getTarget().getName() = "memcpy" + ) and + // All of these memory copy functions have source as the second parameter and size as the third parameter + io_stack_param = mem_copy_func.getArgument(1) and + io_stack_param.getType() instanceof PIO_STACK_LOCATION and + size = mem_copy_func.getArgument(2).getValue() and + size = "36" // Size of the IRP stack entry in bytes in decimal form. (0x24 hex) +select mem_copy_func, "Possible copy of a whole IRP stack entry $@ at $@", io_stack_param, + io_stack_param.toString(), mem_copy_func, mem_copy_func.toString() diff --git a/src/drivers/general/queries/IRPStackEntryCopy/IRPStackEntryCopy.sarif b/src/drivers/general/queries/IRPStackEntryCopy/IRPStackEntryCopy.sarif new file mode 100644 index 00000000..aa2cde90 --- /dev/null +++ b/src/drivers/general/queries/IRPStackEntryCopy/IRPStackEntryCopy.sarif @@ -0,0 +1,383 @@ +{ + "$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/irp-stack-entry-copy", + "name": "cpp/drivers/irp-stack-entry-copy", + "shortDescription": { + "text": "Irp stack entry copy" + }, + "fullDescription": { + "text": "C28114: Copying a whole IRP stack entry leaves certain fields initialized that should be cleared or updated." + }, + "defaultConfiguration": { + "enabled": true, + "level": "warning" + }, + "properties": { + "tags": [ + "correctness", + "wddst" + ], + "description": "C28114: Copying a whole IRP stack entry leaves certain fields initialized that should be cleared or updated.", + "feature.area": "Multiple", + "id": "cpp/drivers/irp-stack-entry-copy", + "impact": "Exploitable Design", + "kind": "problem", + "name": "Irp stack entry copy", + "opaqueid": "CQLD-C28114", + "owner.email": "sdat@microsoft.com", + "platform": "Desktop", + "precision": "medium", + "problem.severity": "warning", + "query-version": "v1", + "repro.text": "", + "scope": "domainspecific", + "security.severity": "Medium" + } + } + ] + }, + "extensions": [ + { + "name": "microsoft/windows-drivers", + "semanticVersion": "1.1.0+090220833fe7953b20e1287167c438367e68d1d1", + "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-22T00:11:46.713+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/irp-stack-entry-copy", + "ruleIndex": 0, + "rule": { + "id": "cpp/drivers/irp-stack-entry-copy", + "index": 0 + }, + "message": { + "text": "Possible copy of a whole IRP stack entry [... + ...](1) at [call to memcpy](2)" + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "driver/driver_snippet.c", + "uriBaseId": "%SRCROOT%", + "index": 0 + }, + "region": { + "startLine": 28, + "startColumn": 5, + "endColumn": 89 + } + } + } + ], + "partialFingerprints": { + "primaryLocationLineHash": "4553eaa36b9212be:1", + "primaryLocationStartColumnFingerprint": "0" + }, + "relatedLocations": [ + { + "id": 1, + "physicalLocation": { + "artifactLocation": { + "uri": "driver/driver_snippet.c", + "uriBaseId": "%SRCROOT%", + "index": 0 + }, + "region": { + "startLine": 28, + "startColumn": 32, + "endColumn": 39 + } + }, + "message": { + "text": "... + ..." + } + }, + { + "id": 2, + "physicalLocation": { + "artifactLocation": { + "uri": "driver/driver_snippet.c", + "uriBaseId": "%SRCROOT%", + "index": 0 + }, + "region": { + "startLine": 28, + "startColumn": 5, + "endColumn": 89 + } + }, + "message": { + "text": "call to memcpy" + } + } + ] + }, + { + "ruleId": "cpp/drivers/irp-stack-entry-copy", + "ruleIndex": 0, + "rule": { + "id": "cpp/drivers/irp-stack-entry-copy", + "index": 0 + }, + "message": { + "text": "Possible copy of a whole IRP stack entry [irpSp](1) at [call to memcpy](2)" + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "driver/driver_snippet.c", + "uriBaseId": "%SRCROOT%", + "index": 0 + }, + "region": { + "startLine": 18, + "startColumn": 5, + "endColumn": 42 + } + } + } + ], + "partialFingerprints": { + "primaryLocationLineHash": "529201672625ca13:1", + "primaryLocationStartColumnFingerprint": "0" + }, + "relatedLocations": [ + { + "id": 1, + "physicalLocation": { + "artifactLocation": { + "uri": "driver/driver_snippet.c", + "uriBaseId": "%SRCROOT%", + "index": 0 + }, + "region": { + "startLine": 18, + "startColumn": 30, + "endColumn": 35 + } + }, + "message": { + "text": "irpSp" + } + }, + { + "id": 2, + "physicalLocation": { + "artifactLocation": { + "uri": "driver/driver_snippet.c", + "uriBaseId": "%SRCROOT%", + "index": 0 + }, + "region": { + "startLine": 18, + "startColumn": 5, + "endColumn": 42 + } + }, + "message": { + "text": "call to memcpy" + } + } + ] + } + ], + "columnKind": "utf16CodeUnits", + "properties": { + "semmle.formatSpecifier": "sarifv2.1.0" + } + } + ] +} \ No newline at end of file diff --git a/src/drivers/general/queries/IRPStackEntryCopy/driver_snippet.c b/src/drivers/general/queries/IRPStackEntryCopy/driver_snippet.c new file mode 100644 index 00000000..905ec3f8 --- /dev/null +++ b/src/drivers/general/queries/IRPStackEntryCopy/driver_snippet.c @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// +// driver_snippet.c +// + +#define SET_DISPATCH 1 +// Template. Not called in this test. +void top_level_call() {} + +void bad_irp_copy( + PIRP irp) +{ + PIO_STACK_LOCATION irpSp; + PIO_STACK_LOCATION nextIrpSp; + irpSp = IoGetCurrentIrpStackLocation(irp); + nextIrpSp = IoGetNextIrpStackLocation(irp); + RtlCopyMemory(nextIrpSp, irpSp, 0x24); + nextIrpSp->Control = 0; +} +void bad_irp_copy2( + PIRP irp) +{ + PIO_STACK_LOCATION irpSp; + PIO_STACK_LOCATION nextIrpSp; + irpSp = IoGetCurrentIrpStackLocation(irp); + nextIrpSp = IoGetNextIrpStackLocation(irp); + RtlCopyMemory(nextIrpSp+4, irpSp+4, FIELD_OFFSET(IO_STACK_LOCATION, DeviceObject)-4); + nextIrpSp->Control = 0; +} +void good_irp_copy( + PIRP irp) +{ + IoCopyCurrentIrpStackLocationToNext(irp); +} diff --git a/src/drivers/test/diff/IRPStackEntryCopy.sarif b/src/drivers/test/diff/IRPStackEntryCopy.sarif new file mode 100644 index 00000000..dea8275e --- /dev/null +++ b/src/drivers/test/diff/IRPStackEntryCopy.sarif @@ -0,0 +1,21 @@ +{ + "all": { + "+": 0, + "-": 0 + }, + "error": { + "+": 0, + "-": 0, + "codes": [] + }, + "warning": { + "+": 0, + "-": 0, + "codes": [] + }, + "note": { + "+": 0, + "-": 0, + "codes": [] + } +} \ No newline at end of file diff --git a/src/windows-driver-suites/ported_driver_ca_checks.qls b/src/windows-driver-suites/ported_driver_ca_checks.qls index b50f59c0..ae1b044f 100644 --- a/src/windows-driver-suites/ported_driver_ca_checks.qls +++ b/src/windows-driver-suites/ported_driver_ca_checks.qls @@ -19,6 +19,7 @@ - drivers/general/queries/IrqlTooLow/IrqlTooLow.ql - drivers/general/queries/IrqlSetTooHigh/IrqlSetTooHigh.ql - drivers/general/queries/IrqlSetTooLow/IrqlSetTooLow.ql + - drivers/general/queries/IRPStackEntryCopy/IRPStackEntryCopy.ql - drivers/general/queries/DriverEntrySaveBuffer/DriverEntrySaveBuffer.ql - drivers/general/queries/CurrentFunctionTypeNotCorrect/CurrentFunctionTypeNotCorrect.ql - drivers/general/queries/IoInitializeTimerCall/IoInitializeTimerCall.ql