Skip to content

Commit

Permalink
Fix 18-1 amendment use-use flow
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelRFairhurst committed Jan 16, 2025
1 parent 0c7b503 commit bd3e06b
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ abstract class ArrayLikeAccess extends Expr {
*/
class ArrayVariableAccess extends ArrayLikeAccess, VariableAccess {
int size;

ArrayVariableAccess() { size = getType().(ArrayType).getArraySize() }

override Variable getElement() { result = getTarget() }
Expand All @@ -47,7 +47,7 @@ class ArrayVariableAccess extends ArrayLikeAccess, VariableAccess {

/**
* Get the size of the object pointed to by a type (pointer or array).
*
*
* Depth of type unwrapping depends on the type. Pointer will be dereferenced only once: the element
* size of `T*` is `sizeof(T)` while the element size of `T**` is `sizeof(T*)`. However, array types
* will be deeply unwrapped, as the pointed to size of `T[][]` is `sizeof(T)`. These processes
Expand Down Expand Up @@ -88,7 +88,17 @@ class CastedToBytePointer extends ArrayLikeAccess, Conversion {

override int getSize() { result = size }

override DataFlow::Node getNode() { result.asConvertedExpr() = this }
override DataFlow::Node getNode() {
// Carefully avoid use-use flow, which would mean any later usage of the original pointer value
// after the cast would be considered a usage of the byte pointer value.
//
// To fix this, we currently assume the value is assigned to a variable, and find that variable
// with `.asDefinition()` like so:
exists(DataFlow::Node conversion |
conversion.asConvertedExpr() = this and
result.asDefinition() = conversion.asExpr()
)
}
}

/**
Expand Down Expand Up @@ -272,4 +282,4 @@ query predicate problems(Expr arrayPointerCreation, string message, Element arra
"Array pointer " + derivedArrayPointerOrPointerOperand.getName() + " points " +
difference.toString() + " " + denomination + " past the end of $@."
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,3 @@
| test.cpp:25:15:25:21 | & ... | Array pointer p14 points 1 element past the end of $@. | test.cpp:2:7:2:8 | l1 | l1 |
| test.cpp:30:15:30:21 | & ... | Array pointer p17 points 1 element past the end of $@. | test.cpp:28:24:28:42 | (unsigned char *)... | cast to btye pointer (unsigned char *)... |
| test.cpp:35:15:35:21 | & ... | Array pointer p20 points 1 element past the end of $@. | test.cpp:33:24:33:43 | (unsigned char *)... | cast to btye pointer (unsigned char *)... |
| test.cpp:43:15:43:23 | & ... | Array pointer p23 points 96 elements past the end of $@. | test.cpp:28:24:28:42 | (unsigned char *)... | cast to btye pointer (unsigned char *)... |
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@ void f1() {

// Casting to a pointer to a differently sized type that isn't char
// invalidates analysis
int l3[3];
long *p21 = (long*)&l1;
long *p21 = (long *)&l1;
void *p22 = &p21[0]; // COMPLIANT
// Not compliant, but we shouldn't detect it, but we do for the wrong reason:
void *p23 = &p21[100]; // NON_COMPLIANT[FALSE_NEGATIVE][FALSE_POSITIVE]
void *p23 = &p21[100]; // NON_COMPLIANT[FALSE_NEGATIVE]
}

0 comments on commit bd3e06b

Please sign in to comment.