Skip to content

Commit 9f517fd

Browse files
balazskebalazske
committed
[clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange
Add some notes and track of bad return value. Reviewed By: steakhal Differential Revision: https://reviews.llvm.org/D107051
1 parent f6748b2 commit 9f517fd

File tree

3 files changed

+106
-30
lines changed

3 files changed

+106
-30
lines changed

‎clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp

+37-8
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
15+
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
1516
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
1617
#include "clang/StaticAnalyzer/Core/Checker.h"
1718
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -79,16 +80,44 @@ void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt *RS,
7980
"Returned pointer value points outside the original object "
8081
"(potential buffer overflow)"));
8182

82-
// FIXME: It would be nice to eventually make this diagnostic more clear,
83-
// e.g., by referencing the original declaration or by saying *why* this
84-
// reference is outside the range.
85-
8683
// Generate a report for this bug.
87-
auto report =
84+
auto Report =
8885
std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N);
89-
90-
report->addRange(RetE->getSourceRange());
91-
C.emitReport(std::move(report));
86+
Report->addRange(RetE->getSourceRange());
87+
88+
const auto ConcreteElementCount = ElementCount.getAs<nonloc::ConcreteInt>();
89+
const auto ConcreteIdx = Idx.getAs<nonloc::ConcreteInt>();
90+
91+
const auto *DeclR = ER->getSuperRegion()->getAs<DeclRegion>();
92+
93+
if (DeclR)
94+
Report->addNote("Original object declared here",
95+
{DeclR->getDecl(), C.getSourceManager()});
96+
97+
if (ConcreteElementCount) {
98+
SmallString<128> SBuf;
99+
llvm::raw_svector_ostream OS(SBuf);
100+
OS << "Original object ";
101+
if (DeclR) {
102+
OS << "'";
103+
DeclR->getDecl()->printName(OS);
104+
OS << "' ";
105+
}
106+
OS << "is an array of " << ConcreteElementCount->getValue() << " '";
107+
ER->getValueType().print(OS,
108+
PrintingPolicy(C.getASTContext().getLangOpts()));
109+
OS << "' objects";
110+
if (ConcreteIdx) {
111+
OS << ", returned pointer points at index " << ConcreteIdx->getValue();
112+
}
113+
114+
Report->addNote(SBuf,
115+
{RetE, C.getSourceManager(), C.getLocationContext()});
116+
}
117+
118+
bugreporter::trackExpressionValue(N, RetE, *Report);
119+
120+
C.emitReport(std::move(Report));
92121
}
93122
}
94123

‎clang/test/Analysis/misc-ps-region-store.m

+2-1
Original file line numberDiff line numberDiff line change
@@ -461,10 +461,11 @@ void element_region_with_symbolic_superregion(int* p) {
461461
// Test returning an out-of-bounds pointer (CWE-466)
462462
//===----------------------------------------------------------------------===//
463463

464-
static int test_cwe466_return_outofbounds_pointer_a[10];
464+
static int test_cwe466_return_outofbounds_pointer_a[10]; // expected-note{{Original object declared here}}
465465
int *test_cwe466_return_outofbounds_pointer() {
466466
int *p = test_cwe466_return_outofbounds_pointer_a+11;
467467
return p; // expected-warning{{Returned pointer value points outside the original object}}
468+
// expected-note@-1{{Original object 'test_cwe466_return_outofbounds_pointer_a' is an array of 10 'int' objects, returned pointer points at index 11}}
468469
}
469470

470471
//===----------------------------------------------------------------------===//

‎clang/test/Analysis/return-ptr-range.cpp

+67-21
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,39 @@
1-
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.ReturnPtrRange -verify %s
2-
3-
int arr[10];
4-
int *ptr;
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.ReturnPtrRange -analyzer-output text -verify %s
52

63
int conjure_index();
74

8-
int *test_element_index_lifetime() {
9-
do {
5+
namespace test_element_index_lifetime {
6+
7+
int arr[10]; // expected-note{{Original object declared here}} expected-note{{Original object declared here}}
8+
int *ptr;
9+
10+
int *test_global_ptr() {
11+
do { // expected-note{{Loop condition is false. Exiting loop}}
1012
int x = conjure_index();
11-
ptr = arr + x;
12-
if (x != 20)
13+
ptr = arr + x; // expected-note{{Value assigned to 'ptr'}}
14+
if (x != 20) // expected-note{{Assuming 'x' is equal to 20}}
15+
// expected-note@-1{{Taking false branch}}
1316
return arr; // no-warning
14-
} while (0);
15-
return ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
17+
} while(0);
18+
return ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
19+
// expected-note@-1{{Returned pointer value points outside the original object (potential buffer overflow)}}
20+
// expected-note@-2{{Original object 'arr' is an array of 10 'int' objects}}
1621
}
1722

18-
int *test_element_index_lifetime_with_local_ptr() {
23+
int *test_local_ptr() {
1924
int *local_ptr;
20-
do {
25+
do { // expected-note{{Loop condition is false. Exiting loop}}
2126
int x = conjure_index();
22-
local_ptr = arr + x;
23-
if (x != 20)
27+
local_ptr = arr + x; // expected-note{{Value assigned to 'local_ptr'}}
28+
if (x != 20) // expected-note{{Assuming 'x' is equal to 20}}
29+
// expected-note@-1{{Taking false branch}}
2430
return arr; // no-warning
25-
} while (0);
26-
return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
31+
} while(0);
32+
return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
33+
// expected-note@-1{{Returned pointer value points outside the original object (potential buffer overflow)}}
34+
// expected-note@-2{{Original object 'arr' is an array of 10 'int' objects}}
35+
}
36+
2737
}
2838

2939
template <typename T, int N>
@@ -55,17 +65,53 @@ void use_iterable_object() {
5565

5666
template <int N>
5767
class BadIterable {
58-
int buffer[N];
68+
int buffer[N]; // expected-note{{Original object declared here}}
5969
int *start, *finish;
6070

6171
public:
62-
BadIterable() : start(buffer), finish(buffer + N) {}
72+
BadIterable() : start(buffer), finish(buffer + N) {} // expected-note{{Value assigned to 'iter.finish'}}
6373

6474
int* begin() { return start; }
65-
int* end() { return finish + 1; } // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
75+
int* end() { return finish + 1; } // expected-warning{{Returned pointer value points outside the original object}}
76+
// expected-note@-1{{Returned pointer value points outside the original object}}
77+
// expected-note@-2{{Original object 'buffer' is an array of 20 'int' objects, returned pointer points at index 21}}
6678
};
6779

6880
void use_bad_iterable_object() {
69-
BadIterable<20> iter;
70-
iter.end();
81+
BadIterable<20> iter; // expected-note{{Calling default constructor for 'BadIterable<20>'}}
82+
// expected-note@-1{{Returning from default constructor for 'BadIterable<20>'}}
83+
iter.end(); // expected-note{{Calling 'BadIterable::end'}}
7184
}
85+
86+
int *test_idx_sym(int I) {
87+
static int arr[10]; // expected-note{{Original object declared here}} expected-note{{'arr' initialized here}}
88+
89+
if (I != 11) // expected-note{{Assuming 'I' is equal to 11}}
90+
// expected-note@-1{{Taking false branch}}
91+
return arr;
92+
return arr + I; // expected-warning{{Returned pointer value points outside the original object}}
93+
// expected-note@-1{{Returned pointer value points outside the original object}}
94+
// expected-note@-2{{Original object 'arr' is an array of 10 'int' objects, returned pointer points at index 11}}
95+
}
96+
97+
namespace test_array_of_struct {
98+
99+
struct Data {
100+
int A;
101+
char *B;
102+
};
103+
104+
Data DataArr[10]; // expected-note{{Original object declared here}}
105+
106+
Data *test_struct_array() {
107+
int I = conjure_index();
108+
if (I != 11) // expected-note{{Assuming 'I' is equal to 11}}
109+
// expected-note@-1{{Taking false branch}}
110+
return DataArr;
111+
return DataArr + I; // expected-warning{{Returned pointer value points outside the original object}}
112+
// expected-note@-1{{Returned pointer value points outside the original object}}
113+
// expected-note@-2{{Original object 'DataArr' is an array of 10 'test_array_of_struct::Data' objects, returned pointer points at index 11}}
114+
}
115+
116+
}
117+

0 commit comments

Comments
 (0)