From ee6bc15edecb6f809aa13b24d2c38f4b09786c58 Mon Sep 17 00:00:00 2001 From: Lianbo Jiang Date: Wed, 6 Mar 2024 17:18:32 +0800 Subject: [PATCH] Fix the overrun issue reported by static application security testing 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 --- snappy_test_tool.cc | 15 ++++++++------- snappy_unittest.cc | 15 ++++++++------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/snappy_test_tool.cc b/snappy_test_tool.cc index a7c779b..3ded89c 100644 --- a/snappy_test_tool.cc +++ b/snappy_test_tool.cc @@ -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_); @@ -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_)); } @@ -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 diff --git a/snappy_unittest.cc b/snappy_unittest.cc index e57b13d..11fb562 100644 --- a/snappy_unittest.cc +++ b/snappy_unittest.cc @@ -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_); @@ -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_)); } @@ -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