Skip to content

M0-2-1: make into split and shared query #637

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

Merged
merged 15 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import cpp
import codingstandards.c.cert
import codingstandards.c.Pointers
import codingstandards.cpp.Pointers
import codingstandards.cpp.dataflow.TaintTracking
import ScaledIntegerPointerArithmeticFlow::PathGraph

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,177 +12,11 @@

import cpp
import codingstandards.c.cert
import codingstandards.c.Pointers
import codingstandards.c.Variable
import codingstandards.cpp.dataflow.DataFlow
import semmle.code.cpp.pointsto.PointsTo
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
import codingstandards.cpp.rules.donotpassaliasedpointertorestrictqualifiedparamshared.DoNotPassAliasedPointerToRestrictQualifiedParamShared

/**
* A function that has a parameter with a restrict-qualified pointer type.
*/
class FunctionWithRestrictParameters extends Function {
Parameter restrictPtrParam;

FunctionWithRestrictParameters() {
restrictPtrParam.getUnspecifiedType() instanceof PointerOrArrayType and
(
restrictPtrParam.getType().hasSpecifier(["restrict"]) and
restrictPtrParam = this.getAParameter()
or
this.hasGlobalName(["strcpy", "strncpy", "strcat", "strncat", "memcpy"]) and
restrictPtrParam = this.getParameter([0, 1])
or
this.hasGlobalName(["strcpy_s", "strncpy_s", "strcat_s", "strncat_s", "memcpy_s"]) and
restrictPtrParam = this.getParameter([0, 2])
or
this.hasGlobalName(["strtok_s"]) and
restrictPtrParam = this.getAParameter()
or
this.hasGlobalName(["printf", "printf_s", "scanf", "scanf_s"]) and
restrictPtrParam = this.getParameter(0)
or
this.hasGlobalName(["sprintf", "sprintf_s", "snprintf", "snprintf_s"]) and
restrictPtrParam = this.getParameter(3)
)
}

Parameter getARestrictPtrParam() { result = restrictPtrParam }
}

/**
* A call to a function that has a parameter with a restrict-qualified pointer type.
*/
class CallToFunctionWithRestrictParameters extends FunctionCall {
CallToFunctionWithRestrictParameters() {
this.getTarget() instanceof FunctionWithRestrictParameters
}

Expr getARestrictPtrArg() {
result =
this.getArgument(this.getTarget()
.(FunctionWithRestrictParameters)
.getARestrictPtrParam()
.getIndex())
}

Expr getAPtrArg(int index) {
result = this.getArgument(index) and
pointerValue(result)
}

Expr getAPossibleSizeArg() {
exists(Parameter param |
param = this.getTarget().(FunctionWithRestrictParameters).getAParameter() and
param.getUnderlyingType() instanceof IntegralType and
// exclude __builtin_object_size
not result.(FunctionCall).getTarget() instanceof BuiltInFunction and
result = this.getArgument(param.getIndex())
)
}
}

/**
* A `PointsToExpr` that is an argument of a pointer-type in a `CallToFunctionWithRestrictParameters`
*/
class CallToFunctionWithRestrictParametersArgExpr extends Expr {
int paramIndex;

CallToFunctionWithRestrictParametersArgExpr() {
this = any(CallToFunctionWithRestrictParameters call).getAPtrArg(paramIndex)
class DoNotPassAliasedPointerToRestrictQualifiedParamQuery extends DoNotPassAliasedPointerToRestrictQualifiedParamSharedSharedQuery
{
DoNotPassAliasedPointerToRestrictQualifiedParamQuery() {
this = Pointers3Package::doNotPassAliasedPointerToRestrictQualifiedParamQuery()
}

int getParamIndex() { result = paramIndex }
}

int getStatedValue(Expr e) {
// `upperBound(e)` defaults to `exprMaxVal(e)` when `e` isn't analyzable. So to get a meaningful
// result in this case we pick the minimum value obtainable from dataflow and range analysis.
result =
upperBound(e)
.minimum(min(Expr source | DataFlow::localExprFlow(source, e) | source.getValue().toInt()))
}

int getPointerArithmeticOperandStatedValue(CallToFunctionWithRestrictParametersArgExpr expr) {
result = getStatedValue(expr.(PointerArithmeticExpr).getOperand())
or
// edge-case: &(array[index]) expressions
result = getStatedValue(expr.(AddressOfExpr).getOperand().(PointerArithmeticExpr).getOperand())
or
// fall-back if `expr` is not a pointer arithmetic expression
not expr instanceof PointerArithmeticExpr and
not expr.(AddressOfExpr).getOperand() instanceof PointerArithmeticExpr and
result = 0
}

module PointerValueToRestrictArgConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { pointerValue(source.asExpr()) }

predicate isSink(DataFlow::Node sink) {
exists(CallToFunctionWithRestrictParameters call |
sink.asExpr() = call.getAPtrArg(_).getAChild*()
)
}

predicate isBarrierIn(DataFlow::Node node) {
exists(AddressOfExpr a | node.asExpr() = a.getOperand().getAChild*())
}
}

module PointerValueToRestrictArgFlow = DataFlow::Global<PointerValueToRestrictArgConfig>;

from
CallToFunctionWithRestrictParameters call, CallToFunctionWithRestrictParametersArgExpr arg1,
CallToFunctionWithRestrictParametersArgExpr arg2, int argOffset1, int argOffset2, Expr source1,
Expr source2, string sourceMessage1, string sourceMessage2
where
not isExcluded(call, Pointers3Package::doNotPassAliasedPointerToRestrictQualifiedParamQuery()) and
arg1 = call.getARestrictPtrArg() and
arg2 = call.getAPtrArg(_) and
// enforce ordering to remove permutations if multiple restrict-qualified args exist
(not arg2 = call.getARestrictPtrArg() or arg2.getParamIndex() > arg1.getParamIndex()) and
(
// check if two pointers address the same object
PointerValueToRestrictArgFlow::flow(DataFlow::exprNode(source1),
DataFlow::exprNode(arg1.getAChild*())) and
(
// one pointer value flows to both args
PointerValueToRestrictArgFlow::flow(DataFlow::exprNode(source1),
DataFlow::exprNode(arg2.getAChild*())) and
sourceMessage1 = "$@" and
sourceMessage2 = "source" and
source1 = source2
or
// there are two separate values that flow from an AddressOfExpr of the same target
getAddressOfExprTargetBase(source1) = getAddressOfExprTargetBase(source2) and
PointerValueToRestrictArgFlow::flow(DataFlow::exprNode(source2),
DataFlow::exprNode(arg2.getAChild*())) and
sourceMessage1 = "a pair of address-of expressions ($@, $@)" and
sourceMessage2 = "addressof1" and
not source1 = source2
)
) and
// get the offset of the pointer arithmetic operand (or '0' if there is none)
argOffset1 = getPointerArithmeticOperandStatedValue(arg1) and
argOffset2 = getPointerArithmeticOperandStatedValue(arg2) and
(
// case 1: the pointer args are the same.
// (definite aliasing)
argOffset1 = argOffset2
or
// case 2: the pointer args are different, a size arg exists,
// and the size arg is greater than the difference between the offsets.
// (potential aliasing)
exists(Expr sizeArg |
sizeArg = call.getAPossibleSizeArg() and
getStatedValue(sizeArg) > (argOffset1 - argOffset2).abs()
)
or
// case 3: the pointer args are different, and a size arg does not exist
// (potential aliasing)
not exists(call.getAPossibleSizeArg())
)
select call,
"Call to '" + call.getTarget().getName() + "' passes an $@ to a $@ (pointer value derived from " +
sourceMessage1 + ".", arg2, "aliased pointer", arg1, "restrict-qualified parameter", source1,
sourceMessage2, source2, "addressof2"
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import cpp
import codingstandards.cpp.dataflow.DataFlow
import semmle.code.cpp.controlflow.Dominance
import codingstandards.c.cert
import codingstandards.c.Variable
import codingstandards.cpp.Variable

/**
* An `Expr` that is an assignment or initialization to a restrict-qualified pointer-type variable.
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
c/common/test/rules/donotpassaliasedpointertorestrictqualifiedparamshared/DoNotPassAliasedPointerToRestrictQualifiedParamShared.ql
2 changes: 1 addition & 1 deletion c/common/src/codingstandards/c/OutOfBounds.qll
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import cpp
import codingstandards.c.Pointers
import codingstandards.cpp.Pointers
import codingstandards.c.Variable
import codingstandards.cpp.Allocations
import codingstandards.cpp.Overflow
Expand Down
14 changes: 0 additions & 14 deletions c/common/src/codingstandards/c/Variable.qll
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,6 @@ class FlexibleArrayMemberCandidate extends MemberVariable {
}
}

/**
* Returns the target variable of a `VariableAccess`.
* If the access is a field access, then the target is the `Variable` of the qualifier.
* If the access is an array access, then the target is the array base.
*/
Variable getAddressOfExprTargetBase(AddressOfExpr expr) {
result = expr.getOperand().(ValueFieldAccess).getQualifier().(VariableAccess).getTarget()
or
not expr.getOperand() instanceof ValueFieldAccess and
result = expr.getOperand().(VariableAccess).getTarget()
or
result = expr.getOperand().(ArrayExpr).getArrayBase().(VariableAccess).getTarget()
}

/**
* A struct that contains a flexible array member
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// GENERATED FILE - DO NOT MODIFY
import codingstandards.cpp.rules.donotpassaliasedpointertorestrictqualifiedparamshared.DoNotPassAliasedPointerToRestrictQualifiedParamShared

class TestFileQuery extends DoNotPassAliasedPointerToRestrictQualifiedParamSharedSharedQuery,
TestQuery
{ }
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
#include <stddef.h>
#include <stdio.h>
#include <string.h>

int *restrict g1;
int *restrict g2;
int *restrict g1_1;
int *g2_1;

struct s1 {
int x, y, z;
};
struct s1 v1;

void test_global_local() {
int *restrict i1 = g1; // COMPLIANT
int *restrict i2 = g2; // COMPLIANT
int *restrict i3 = i2; // NON_COMPLIANT
g1 = g2; // NON_COMPLIANT
i1 = i2; // NON_COMPLIANT
{
int *restrict i4;
int *restrict i5;
int *restrict i6;
i4 = g1; // COMPLIANT
i4 = (void *)0; // COMPLIANT
i5 = g1; // NON_COMPLIANT - block rather than statement scope matters
i4 = g1; // NON_COMPLIANT
i6 = g2; // COMPLIANT
}
}

void test_global_local_1() {
g1_1 = g2_1; // COMPLIANT
}

void test_structs() {
struct s1 *restrict p1 = &v1;
int *restrict px = &v1.x; // NON_COMPLIANT
{
int *restrict py;
int *restrict pz;
py = &v1.y; // COMPLIANT
py = (int *)0;
pz = &v1.z; // NON_COMPLIANT - block rather than statement scope matters
py = &v1.y; // NON_COMPLIANT
}
}

void copy(int *restrict p1, int *restrict p2, size_t s) {
for (size_t i = 0; i < s; ++i) {
p2[i] = p1[i];
}
}

void test_restrict_params() {
int i1 = 1;
int i2 = 2;
copy(&i1, &i1, 1); // NON_COMPLIANT
copy(&i1, &i2, 1); // COMPLIANT

int x[10];
int *px = &x[0];
copy(&x[0], &x[1], 1); // COMPLIANT - non overlapping
copy(&x[0], &x[1], 2); // NON_COMPLIANT - overlapping
copy(&x[0], (int *)x[0], 1); // COMPLIANT - non overlapping
copy(&x[0], px, 1); // NON_COMPLIANT - overlapping
}

void test_strcpy() {
char s1[] = "my test string";
char s2[] = "my other string";
strcpy(&s1, &s1 + 3); // NON_COMPLIANT
strcpy(&s2, &s1); // COMPLIANT
}

void test_memcpy() {
char s1[] = "my test string";
char s2[] = "my other string";
memcpy(&s1, &s1 + 3, 5); // NON_COMPLIANT
memcpy(&s2, &s1 + 3, 5); // COMPLIANT
}

void test_memmove() {
char s1[] = "my test string";
char s2[] = "my other string";
memmove(&s1, &s1 + 3, 5); // COMPLIANT - memmove is allowed to overlap
memmove(&s2, &s1 + 3, 5); // COMPLIANT
}

void test_scanf() {
char s1[200] = "%10s";
scanf(&s1, &s1 + 4); // NON_COMPLIANT
}

// TODO also consider the following:
// strncpy(), strncpy_s()
// strcat(), strcat_s()
// strncat(), strncat_s()
// strtok_s()
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import cpp
import codingstandards.c.misra
import codingstandards.c.Pointers
import codingstandards.cpp.Pointers

from CStyleCast cast, Type type, Type newType
where
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import cpp
import codingstandards.c.misra
import codingstandards.c.Pointers
import codingstandards.cpp.Pointers
import codingstandards.cpp.Type

from Cast cast, Type type, Type newType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import cpp
import codingstandards.c.misra
import codingstandards.c.Pointers
import codingstandards.cpp.Pointers

from CStyleCast cast, Type baseTypeFrom, Type baseTypeTo
where
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import cpp
import codingstandards.c.misra
import codingstandards.c.Pointers
import codingstandards.cpp.Pointers

from CStyleCast cast, Type typeFrom, Type typeTo
where
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import cpp
import codingstandards.c.misra
import codingstandards.c.Pointers
import codingstandards.cpp.Pointers

from Cast cast, VoidPointerType type, PointerToObjectType newType
where
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import cpp
import codingstandards.c.misra
import codingstandards.c.Pointers
import codingstandards.cpp.Pointers

from CStyleCast cast, Type typeFrom, Type typeTo
where
Expand Down
Loading
Loading