Skip to content

Commit

Permalink
Fix the overrun issue reported by static application security testing
Browse files Browse the repository at this point in the history
The current issue is reported by the SAST(Static Application Security
Testing) as below:

  Error: OVERRUN:
  snappy_unittest.cc:95: return_constant: Function call "sysconf(_SC_PAGESIZE)" may return -1.
  snappy_unittest.cc:95: assignment: Assigning: "page_size" = "sysconf(_SC_PAGESIZE)". The value of "page_size" is now 18446744073709551615.
  snappy_unittest.cc:97: overrun-buffer-arg: Calling "mprotect" with "this->protected_page_" and "page_size" is suspicious because of the very large index, 18446744073709551615. The index may be due to a negative parameter being interpreted as unsigned.
  #   95|       const size_t page_size = sysconf(_SC_PAGESIZE);
  #   96|       // Undo the mprotect.
  #   97|->     CHECK_EQ(0, mprotect(protected_page_, page_size, PROT_READ|PROT_WRITE));
  #   98|       CHECK_EQ(0, munmap(mem_, alloc_size_));
  #   99|     }

Let's set the page size to 4096, if the invoking sysconf(_SC_PAGESIZE)
failed, otherwise still use the actual value of sysconf(_SC_PAGESIZE).
In addition, also save its value in the constructor function in order to
use it again in the deconstructor function, that can avoid calling the
sysconf(_SC_PAGESIZE) twice.

(Did the same changes in snappy_test_tool.cc).

Signed-off-by: Lianbo Jiang <[email protected]>
  • Loading branch information
lian-bo committed Mar 6, 2024
1 parent 27f34a5 commit ee6bc15
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 14 deletions.
15 changes: 8 additions & 7 deletions snappy_test_tool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,12 @@ namespace {
class DataEndingAtUnreadablePage {
public:
explicit DataEndingAtUnreadablePage(const std::string& s) {
const size_t page_size = sysconf(_SC_PAGESIZE);
long page_size = sysconf(_SC_PAGESIZE);
page_size_ = (size_t)(page_size < 0 ? 4096 : page_size);
const size_t size = s.size();
// Round up space for string to a multiple of page_size.
size_t space_for_string = (size + page_size - 1) & ~(page_size - 1);
alloc_size_ = space_for_string + page_size;
// Round up space for string to a multiple of page_size_.
size_t space_for_string = (size + page_size_ - 1) & ~(page_size_ - 1);
alloc_size_ = space_for_string + page_size_;
mem_ = mmap(NULL, alloc_size_,
PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
CHECK_NE(MAP_FAILED, mem_);
Expand All @@ -91,13 +92,12 @@ class DataEndingAtUnreadablePage {
data_ = dst;
size_ = size;
// Make guard page unreadable.
CHECK_EQ(0, mprotect(protected_page_, page_size, PROT_NONE));
CHECK_EQ(0, mprotect(protected_page_, page_size_, PROT_NONE));
}

~DataEndingAtUnreadablePage() {
const size_t page_size = sysconf(_SC_PAGESIZE);
// Undo the mprotect.
CHECK_EQ(0, mprotect(protected_page_, page_size, PROT_READ|PROT_WRITE));
CHECK_EQ(0, mprotect(protected_page_, page_size_, PROT_READ|PROT_WRITE));
CHECK_EQ(0, munmap(mem_, alloc_size_));
}

Expand All @@ -110,6 +110,7 @@ class DataEndingAtUnreadablePage {
char* protected_page_;
const char* data_;
size_t size_;
size_t page_size_;
};

#else // HAVE_FUNC_MMAP && HAVE_FUNC_SYSCONF
Expand Down
15 changes: 8 additions & 7 deletions snappy_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,12 @@ namespace {
class DataEndingAtUnreadablePage {
public:
explicit DataEndingAtUnreadablePage(const std::string& s) {
const size_t page_size = sysconf(_SC_PAGESIZE);
long page_size = sysconf(_SC_PAGESIZE);
page_size_ = (size_t)(page_size < 0 ? 4096 : page_size);
const size_t size = s.size();
// Round up space for string to a multiple of page_size.
size_t space_for_string = (size + page_size - 1) & ~(page_size - 1);
alloc_size_ = space_for_string + page_size;
// Round up space for string to a multiple of page_size_.
size_t space_for_string = (size + page_size_ - 1) & ~(page_size_ - 1);
alloc_size_ = space_for_string + page_size_;
mem_ = mmap(NULL, alloc_size_,
PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
CHECK_NE(MAP_FAILED, mem_);
Expand All @@ -75,13 +76,12 @@ class DataEndingAtUnreadablePage {
data_ = dst;
size_ = size;
// Make guard page unreadable.
CHECK_EQ(0, mprotect(protected_page_, page_size, PROT_NONE));
CHECK_EQ(0, mprotect(protected_page_, page_size_, PROT_NONE));
}

~DataEndingAtUnreadablePage() {
const size_t page_size = sysconf(_SC_PAGESIZE);
// Undo the mprotect.
CHECK_EQ(0, mprotect(protected_page_, page_size, PROT_READ|PROT_WRITE));
CHECK_EQ(0, mprotect(protected_page_, page_size_, PROT_READ|PROT_WRITE));
CHECK_EQ(0, munmap(mem_, alloc_size_));
}

Expand All @@ -94,6 +94,7 @@ class DataEndingAtUnreadablePage {
char* protected_page_;
const char* data_;
size_t size_;
size_t page_size_;
};

#else // HAVE_FUNC_MMAP) && HAVE_FUNC_SYSCONF
Expand Down

0 comments on commit ee6bc15

Please sign in to comment.