From 91752e5307c03248c4eec1e712b1526c2305f5d4 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 26 Jan 2026 15:00:00 +0000 Subject: [PATCH 1/5] C++: Enforce uniqueness in 'getVariableSize'. --- cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll b/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll index 675090ad9584..5dfeb8f3137c 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll @@ -62,11 +62,13 @@ private Class getRootType(FieldAccess fa) { * unspecified type of `v` is a `ReferenceType`. */ private int getVariableSize(Variable v) { - exists(Type t | - t = v.getUnspecifiedType() and - not t instanceof ReferenceType and - result = t.getSize() - ) + result = + unique(Type t | + t = v.getUnspecifiedType() and + not t instanceof ReferenceType + | + t.getSize() + ) } /** From 13a5249a9de2f02d6f779a1018763e2c3037f4e5 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 26 Jan 2026 15:00:58 +0000 Subject: [PATCH 2/5] C++: Enforce uniqueness in the other branch of 'getSize'. --- cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll b/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll index 5dfeb8f3137c..e641e50532f2 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll @@ -81,30 +81,32 @@ private int getSize(VariableAccess va) { not v instanceof Field and result = getVariableSize(v) or - exists(Class c, int trueSize | - // Otherwise, we find the "outermost" object and compute the size - // as the difference between the size of the type of the "outermost - // object" and the offset of the field relative to that type. - // For example, consider the following structs: - // ``` - // struct S { - // uint32_t x; - // uint32_t y; - // }; - // struct S2 { - // S s; - // uint32_t z; - // }; - // ``` - // Given an object `S2 s2` the size of the buffer `&s2.s.y` - // is the size of the base object type (i.e., `S2`) minutes the offset - // of `y` relative to the type `S2` (i.e., `4`). So the size of the - // buffer is `12 - 4 = 8`. - c = getRootType(va) and - // we calculate the size based on the last field, to avoid including any padding after it - trueSize = max(Field f | | f.getOffsetInClass(c) + getVariableSize(f)) and - result = trueSize - v.(Field).getOffsetInClass(c) - ) + result = + unique(Class c, int trueSize | + // Otherwise, we find the "outermost" object and compute the size + // as the difference between the size of the type of the "outermost + // object" and the offset of the field relative to that type. + // For example, consider the following structs: + // ``` + // struct S { + // uint32_t x; + // uint32_t y; + // }; + // struct S2 { + // S s; + // uint32_t z; + // }; + // ``` + // Given an object `S2 s2` the size of the buffer `&s2.s.y` + // is the size of the base object type (i.e., `S2`) minutes the offset + // of `y` relative to the type `S2` (i.e., `4`). So the size of the + // buffer is `12 - 4 = 8`. + c = getRootType(va) and + // we calculate the size based on the last field, to avoid including any padding after it + trueSize = max(Field f | | f.getOffsetInClass(c) + getVariableSize(f)) + | + trueSize - v.(Field).getOffsetInClass(c) + ) ) } From aed0e688f538340684f2953b1e5ce9ec6a61419d Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 26 Jan 2026 15:01:40 +0000 Subject: [PATCH 3/5] C++: Remove uniqueness since it is enforced earlier now. --- cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll b/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll index e641e50532f2..405eb44c7a95 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll @@ -120,12 +120,8 @@ private int isSource(Expr bufferExpr, Element why) { exists(Variable bufferVar | bufferVar = bufferExpr.(VariableAccess).getTarget() | // buffer is a fixed size array exists(bufferVar.getUnspecifiedType().(ArrayType).getSize()) and - result = - unique(int size | // more generous than .getSize() itself, when the array is a class field or similar. - size = getSize(bufferExpr) - | - size - ) and + // more generous than .getSize() itself, when the array is a class field or similar. + result = getSize(bufferExpr) and why = bufferVar and not memberMayBeVarSize(_, bufferVar) and not exists(BuiltInOperationBuiltInOffsetOf offsetof | offsetof.getAChild*() = bufferExpr) and From 544015d0a64bacda4001fdd2ef12ef715cd612b8 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 26 Jan 2026 19:12:14 +0000 Subject: [PATCH 4/5] C++: Add change note. --- cpp/ql/lib/change-notes/2026-01-26-buffer-overflow-fps.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/lib/change-notes/2026-01-26-buffer-overflow-fps.md diff --git a/cpp/ql/lib/change-notes/2026-01-26-buffer-overflow-fps.md b/cpp/ql/lib/change-notes/2026-01-26-buffer-overflow-fps.md new file mode 100644 index 000000000000..ea9a5ccf7988 --- /dev/null +++ b/cpp/ql/lib/change-notes/2026-01-26-buffer-overflow-fps.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `Buffer.qll` library will no longer report incorrect buffer sizes on certain malformed databases. As a result, the queries `cpp/static-buffer-overflow`, `cpp/overflow-buffer`, `cpp/badly-bounded-write`, `cpp/overrunning-write`, `cpp/overrunning-write-with-float`, and `cpp/very-likely-overrunning-write` will report fewer false positives on such databases. \ No newline at end of file From 980c4cf5f4ce55bd7063a750c0655e509dfb9ba5 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 26 Jan 2026 19:12:38 +0000 Subject: [PATCH 5/5] Update cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll b/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll index 405eb44c7a95..0ea41672b5f1 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll @@ -98,7 +98,7 @@ private int getSize(VariableAccess va) { // }; // ``` // Given an object `S2 s2` the size of the buffer `&s2.s.y` - // is the size of the base object type (i.e., `S2`) minutes the offset + // is the size of the base object type (i.e., `S2`) minus the offset // of `y` relative to the type `S2` (i.e., `4`). So the size of the // buffer is `12 - 4 = 8`. c = getRootType(va) and