Skip to content

Implement IntegerOverflow package #263

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 37 commits into from
Mar 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
23065e4
IntegerOverflow: Create an overflow library
lcartey Mar 19, 2023
0ffa4b1
IntegerOverflow: Add package files.
lcartey Mar 20, 2023
03cb601
IntegerOverflow: Add query for INT30-C
lcartey Mar 20, 2023
c4ff2de
IntegerOverflow: Implement INT33-C
lcartey Mar 20, 2023
195cc3f
IntegerOverflow: Implement Rule 12.4.
lcartey Mar 20, 2023
9580891
Fix typo.
lcartey Mar 20, 2023
e9c0c18
IntegerOverflow: Do not exclude casted expressions
lcartey Mar 20, 2023
d64b125
Macro: Add classes for library vs user macro.
lcartey Mar 20, 2023
a845f38
IntegerOverflow: Exclude macro results
lcartey Mar 20, 2023
3f2db7a
IntegerOverflow: clarify valid post-check
lcartey Mar 20, 2023
37df9a6
IntegerOverflow: Implement INT32-C
lcartey Mar 21, 2023
6dd30df
IntegerOverflow: Support MulExpr
lcartey Mar 21, 2023
5b9e572
INT32-C: Report issues in guards
lcartey Mar 21, 2023
b51dc68
IntegerOverflow: Implement INT31-C
lcartey Mar 21, 2023
9dfc200
INT31-C: Exclude stdbool.h
lcartey Mar 21, 2023
7f672bf
INT31-C: Add better message for typedefs
lcartey Mar 21, 2023
9afa81e
INT31-C: Improve error message to include range
lcartey Mar 21, 2023
417514c
IntegerOverflow: Implement INT35-C
lcartey Mar 21, 2023
f7951fb
IntegerOverflow: Improve descriptions.
lcartey Mar 21, 2023
6d471f2
IntegerOverflow: Update help
lcartey Mar 21, 2023
f5bfa52
Regenerate help files to resolve format issue
Mar 23, 2023
3cede45
Revert "Regenerate help files to resolve format issue"
Mar 23, 2023
1bdbdcd
Fix rule help format issue
Mar 23, 2023
57cf6ec
Merge branch 'main' into lcartey/integer-overflow
Mar 23, 2023
697e2e2
IntegerOverflow: Expand supported operations
lcartey Mar 24, 2023
bf37105
IntegerOverflow: Support divide and remainder
lcartey Mar 24, 2023
d3dc330
IntegerOverflow: Further restrictions
lcartey Mar 24, 2023
6490705
INT32-C: Add tests for ++,--
lcartey Mar 24, 2023
8a67704
IntegerOverflow: INT30-C to medium
lcartey Mar 24, 2023
cb38c60
Merge branch 'main' into lcartey/integer-overflow
lcartey Mar 24, 2023
e4b6417
INT33-C: Include assign div/mod
lcartey Mar 24, 2023
d5b7fe5
Fix typo.
lcartey Mar 24, 2023
b455965
Merge branch 'main' into lcartey/integer-overflow
lcartey Mar 27, 2023
3a33a22
IntegerOverflow: recognise safe crements
lcartey Mar 27, 2023
ba3bf2b
IntgerOverflow: Expose getting a valid post check.
lcartey Mar 27, 2023
68117b7
IntegerOverflow: Support for more guards
lcartey Mar 27, 2023
a2c8fe3
Add change note
lcartey Mar 27, 2023
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
249 changes: 249 additions & 0 deletions c/cert/src/rules/INT30-C/UnsignedIntegerOperationsWrapAround.md

Large diffs are not rendered by default.

38 changes: 38 additions & 0 deletions c/cert/src/rules/INT30-C/UnsignedIntegerOperationsWrapAround.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* @id c/cert/unsigned-integer-operations-wrap-around
* @name INT30-C: Ensure that unsigned integer operations do not wrap
* @description Unsigned integer expressions do not strictly overflow, but instead wrap around in a
* modular way. If the size of the type is not sufficient, this can happen
* unexpectedly.
* @kind problem
* @precision medium
* @problem.severity error
* @tags external/cert/id/int30-c
* correctness
* security
* external/cert/obligation/rule
*/

import cpp
import codingstandards.c.cert
import codingstandards.cpp.Overflow
import semmle.code.cpp.controlflow.Guards
import semmle.code.cpp.valuenumbering.GlobalValueNumbering

from InterestingOverflowingOperation op
where
not isExcluded(op, IntegerOverflowPackage::unsignedIntegerOperationsWrapAroundQuery()) and
op.getType().getUnderlyingType().(IntegralType).isUnsigned() and
// Not within a guard condition
not exists(GuardCondition gc | gc.getAChild*() = op) and
// Not guarded by a check, where the check is not an invalid overflow check
not op.hasValidPreCheck() and
// Is not checked after the operation
not op.hasValidPostCheck() and
// Permitted by exception 3
not op instanceof LShiftExpr and
// Permitted by exception 2 - zero case is handled in separate query
not op instanceof DivExpr and
not op instanceof RemExpr
select op,
"Operation " + op.getOperator() + " of type " + op.getType().getUnderlyingType() + " may wrap."
361 changes: 361 additions & 0 deletions c/cert/src/rules/INT31-C/IntegerConversionCausesDataLoss.md

Large diffs are not rendered by default.

107 changes: 107 additions & 0 deletions c/cert/src/rules/INT31-C/IntegerConversionCausesDataLoss.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/**
* @id c/cert/integer-conversion-causes-data-loss
* @name INT31-C: Ensure that integer conversions do not result in lost or misinterpreted data
* @description Converting an integer value to another integer type with a different sign or size
* can lead to data loss or misinterpretation of the value.
* @kind problem
* @precision medium
* @problem.severity error
* @tags external/cert/id/int31-c
* correctness
* external/cert/obligation/rule
*/

import cpp
import codingstandards.c.cert

class IntegerConversion extends Expr {
private IntegralType castedToType;
private Expr preConversionExpr;

IntegerConversion() {
// This is an explicit cast
castedToType = this.(Cast).getType().getUnspecifiedType() and
preConversionExpr = this.(Cast).getExpr()
or
// Functions that internally cast an argument to unsigned char
castedToType instanceof UnsignedCharType and
this = preConversionExpr and
exists(FunctionCall call, string name | call.getTarget().hasGlobalOrStdName(name) |
name = ["ungetc", "fputc"] and
this = call.getArgument(0)
or
name = ["memset", "memchr"] and
this = call.getArgument(1)
or
name = "memset_s" and
this = call.getArgument(2)
)
}

Expr getPreConversionExpr() { result = preConversionExpr }

Type getCastedToType() { result = castedToType }
}

bindingset[value]
predicate withinIntegralRange(IntegralType typ, float value) {
exists(float lb, float ub, float limit |
limit = 2.pow(8 * typ.getSize()) and
(
if typ.isUnsigned()
then (
lb = 0 and ub = limit - 1
) else (
lb = -limit / 2 and
ub = (limit / 2) - 1
)
) and
value >= lb and
value <= ub
)
}

from
IntegerConversion c, Expr preConversionExpr, Type castedToType, Type castedFromType,
IntegralType unspecifiedCastedFromType, string typeFromMessage, float preConversionLowerBound,
float preConversionUpperBound, float typeLowerBound, float typeUpperBound
where
not isExcluded(c, IntegerOverflowPackage::integerConversionCausesDataLossQuery()) and
preConversionExpr = c.getPreConversionExpr() and
castedFromType = preConversionExpr.getType() and
// Casting from an integral type
unspecifiedCastedFromType = castedFromType.getUnspecifiedType() and
// Casting to an integral type
castedToType = c.getCastedToType() and
// Get the upper/lower bound of the pre-conversion expression
preConversionLowerBound = lowerBound(preConversionExpr) and
preConversionUpperBound = upperBound(preConversionExpr) and
// Get the upper/lower bound of the target type
typeLowerBound = typeLowerBound(castedToType) and
typeUpperBound = typeUpperBound(castedToType) and
// Where the result is not within the range of the target type
(
not withinIntegralRange(castedToType, preConversionLowerBound) or
not withinIntegralRange(castedToType, preConversionUpperBound)
) and
// A conversion of `-1` to `time_t` is permitted by the standard
not (
c.getType().getUnspecifiedType().hasName("time_t") and
preConversionExpr.getValue() = "-1"
) and
// Conversion to unsigned char is permitted from the range [SCHAR_MIN..UCHAR_MAX], as those can
// legitimately represent characters
not (
c.getType().getUnspecifiedType() instanceof UnsignedCharType and
lowerBound(preConversionExpr) >= typeLowerBound(any(SignedCharType s)) and
upperBound(preConversionExpr) <= typeUpperBound(any(UnsignedCharType s))
) and
not castedToType instanceof BoolType and
// Create a helpful message
if castedFromType = unspecifiedCastedFromType
then typeFromMessage = castedFromType.toString()
else typeFromMessage = castedFromType + " (" + unspecifiedCastedFromType + ")"
select c,
"Conversion from " + typeFromMessage + " to " + castedToType +
" may cause data loss (casting from range " + preConversionLowerBound + "..." +
preConversionUpperBound + " to range " + typeLowerBound + "..." + typeUpperBound + ")."
Loading