Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IRPStackEntryCopy: CodeQL port of c28114 #120

Merged
merged 9 commits into from
Aug 23, 2024
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
Copying a whole IRP stack entry leaves certain fields initialized that should be cleared or updated
</p>
</overview>
<recommendation>
<p>
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.
</p>
</recommendation>
<example>
<sample src="driver_snippet.c" />
</example>
<references>
<li>
<a href="https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/28114-improper-irp-stack-copy">
C28114
</a>
</li>
</references>
</qhelp>
48 changes: 48 additions & 0 deletions src/drivers/general/queries/IRPStackEntryCopy/IRPStackEntryCopy.ql
Original file line number Diff line number Diff line change
@@ -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()
Loading
Loading