Pull request #3187: vba: Fixing buffer overflow in ole parser

Merge in SNORT/snort3 from ~VIGNVISW/snort3:vba_bufoverflow to master

Squashed commit of the following:

commit b39fed887c6aed62fbf47a42a77b2b1501340e89
Author: Vigneshwari Viswanathan <vignvisw@cisco.com>
Date:   Wed Nov 24 02:23:41 2021 -0500

    vba: Fixing buffer overflow in ole parser

diff --git a/src/decompress/file_olefile.cc b/src/decompress/file_olefile.cc
index 48712e95eaf99a74c2efa5d2f6f5878397486c22..146a739723b0f5df0768ef6e1892b3ffe4a42464 100644 (file)
--- a/src/decompress/file_olefile.cc
+++ b/src/decompress/file_olefile.cc
@@ -251,7 +251,28 @@ int32_t OleFile :: get_mini_fat_offset(int32_t sec_id)
     return byte_offset;
 }
 
-void OleFile :: get_file_data(char* file, uint8_t*& file_data, int32_t& data_len)
+uint32_t OleFile :: find_bytes_to_copy(uint32_t byte_offset, uint32_t data_len,
+                                   uint32_t stream_size, uint16_t sector_size)
+{
+    uint32_t remaining_bytes = stream_size - data_len;
+    uint32_t bytes_to_copy;
+
+    if ((byte_offset + sector_size) > buf_len)
+    {
+        bytes_to_copy = buf_len - byte_offset;
+    }
+    else
+    {
+        bytes_to_copy = sector_size;
+    }
+
+    if  (bytes_to_copy > remaining_bytes)
+        bytes_to_copy = remaining_bytes;
+
+    return bytes_to_copy;
+}
+        
+void OleFile :: get_file_data(char* file, uint8_t*& file_data, uint32_t& data_len)
 {
     FileProperty* node;
     uint16_t sector_size,mini_sector_size;
@@ -263,7 +284,7 @@ void OleFile :: get_file_data(char* file, uint8_t*& file_data, int32_t& data_len
     if (node)
     {
         int32_t starting_sector;
-        int64_t stream_size;
+        uint32_t stream_size;
         sector_type is_fat = FAT_SECTOR;
         uint32_t byte_offset, bytes_to_copy;
         uint8_t* temp_data;
@@ -286,20 +307,9 @@ void OleFile :: get_file_data(char* file, uint8_t*& file_data, int32_t& data_len
                 if (byte_offset > buf_len)
                     return;
 
-                if ((byte_offset + sector_size)> buf_len)
-                {
-                    memcpy(temp_data, (file_buf + byte_offset), (buf_len - byte_offset));
-                    data_len += buf_len - byte_offset;
-                    VBA_DEBUG(vba_data_trace, DEFAULT_TRACE_OPTION_ID, TRACE_INFO_LEVEL,
-                        CURRENT_PACKET, "Returning with partial data of %d bytes for the "
-                        "file %s.\n", data_len, file);
-                    return;
-                }
-                if ((data_len + sector_size) < stream_size)
-                    bytes_to_copy = sector_size;
-                else
-                    bytes_to_copy = stream_size - data_len;
-
+                bytes_to_copy = find_bytes_to_copy(byte_offset, data_len,
+                                    stream_size, sector_size);
+                
                 memcpy(temp_data, (file_buf + byte_offset), bytes_to_copy);
                 temp_data += sector_size;
                 data_len += bytes_to_copy;
@@ -318,19 +328,8 @@ void OleFile :: get_file_data(char* file, uint8_t*& file_data, int32_t& data_len
                 if (byte_offset > buf_len)
                     return;
 
-                if ((byte_offset + mini_sector_size) > buf_len)
-                {
-                    memcpy(temp_data, (file_buf + byte_offset), (buf_len - byte_offset));
-                    data_len += buf_len - byte_offset;
-                    VBA_DEBUG(vba_data_trace, DEFAULT_TRACE_OPTION_ID, TRACE_INFO_LEVEL,
-                        CURRENT_PACKET, "Returning with partial data of %d bytes for "
-                        "the file %s.\n", data_len, file);
-                    return;
-                }
-                if ((data_len + mini_sector_size) < stream_size)
-                    bytes_to_copy = mini_sector_size;
-                else
-                    bytes_to_copy = stream_size - data_len;
+                bytes_to_copy = find_bytes_to_copy(byte_offset, data_len,
+                                    stream_size, mini_sector_size);
 
                 memcpy(temp_data, (file_buf + byte_offset), bytes_to_copy);
                 temp_data += mini_sector_size;
@@ -522,7 +521,7 @@ bool OleFile :: parse_ole_header()
 // The vba code in a VBA macro file begins with the keyword "ATTRIBUT" .This
 // keyword is used to calculate the offset of vba code and is decompressed using
 // RLE algorithm.
-int32_t OleFile :: get_file_offset(const uint8_t* data, int32_t data_len)
+int32_t OleFile :: get_file_offset(const uint8_t* data, uint32_t data_len)
 {
     if (searcher == nullptr)
     {
@@ -535,7 +534,7 @@ int32_t OleFile :: get_file_offset(const uint8_t* data, int32_t data_len)
     return offset;
 }
 
-int32_t cli_readn(const uint8_t*& fd, int32_t& data_len, void* buff, int32_t count)
+int32_t cli_readn(const uint8_t*& fd, uint32_t& data_len, void* buff, int32_t count)
 {
     int32_t i;
 
@@ -563,8 +562,8 @@ int32_t cli_readn(const uint8_t*& fd, int32_t& data_len, void* buff, int32_t cou
 // the output is a sequence of counts of consecutive data values in a row
 // (i.e. "3A2B4C"). This type of data compression is lossless, meaning that
 // when decompressed, all of the original data will be recovered when decoded.
-void OleFile :: decompression(const uint8_t* data, int32_t* data_len, uint8_t*& local_vba_buffer,
-    uint32_t* vba_buffer_offset)
+void OleFile :: decompression(const uint8_t* data, uint32_t& data_len, uint8_t*& local_vba_buffer,
+    uint32_t& vba_buffer_offset)
 {
     int16_t header;
     bool flagCompressed;
@@ -595,17 +594,17 @@ void OleFile :: decompression(const uint8_t* data, int32_t* data_len, uint8_t*&
     }
 
     data += 3;
-    *data_len -= 3;
+    data_len -= 3;
 
     if (flagCompressed == 0)
     {
-        memcpy(&buffer, data, *data_len);
+        memcpy(&buffer, data, data_len);
         return;
     }
 
     pos = 0;
     clean = 1;
-    int32_t size = *data_len;
+    uint32_t size = data_len;
     while (cli_readn(data, size, &flag, 1))
     {
         for (mask = 1; mask < 0x100; mask <<= 1)
@@ -663,12 +662,12 @@ void OleFile :: decompression(const uint8_t* data, int32_t* data_len, uint8_t*&
 
     int32_t decomp_len = strlen((char*)buffer);
 
-    if ((*vba_buffer_offset + decomp_len) > MAX_VBA_BUFFER_LEN)
+    if ((vba_buffer_offset + decomp_len) > MAX_VBA_BUFFER_LEN)
     {
-        decomp_len =  MAX_VBA_BUFFER_LEN - *vba_buffer_offset;
+        decomp_len =  MAX_VBA_BUFFER_LEN - vba_buffer_offset;
     }
-    memcpy((local_vba_buffer + *vba_buffer_offset), buffer, decomp_len);
-    *vba_buffer_offset += decomp_len;
+    memcpy((local_vba_buffer + vba_buffer_offset), buffer, decomp_len);
+    vba_buffer_offset += decomp_len;
 }
 
 // Function to extract the VBA data and send it for RLE decompression.
@@ -682,7 +681,7 @@ void OleFile :: find_and_extract_vba(uint8_t*& vba_buf, uint32_t& vba_buf_len)
     {
         FileProperty* node;
         uint8_t* data = nullptr;
-        int32_t data_len;
+        uint32_t data_len;
         node = it->second;
         ++it;
         if (node->get_file_type() == STREAM)
@@ -707,7 +706,7 @@ void OleFile :: find_and_extract_vba(uint8_t*& vba_buf, uint32_t& vba_buf_len)
                 "offset %d bytes. First %d bytes will be processed\n",
                 node->get_name(), node->get_stream_size(), (offset - 4), data_len);
 
-            decompression(data, &data_len, vba_buf, &vba_buffer_offset);
+            decompression(data, data_len, vba_buf, vba_buffer_offset);
             delete[] data1;
             if ( vba_buffer_offset >= MAX_VBA_BUFFER_LEN)
                 break;

diff --git a/src/decompress/file_olefile.h b/src/decompress/file_olefile.h
index cd020f0eb0ff799deffd14387243ee0ef90da01c..0d2e407a7170a571bf44a553d424c195955aaac9 100644 (file)
--- a/src/decompress/file_olefile.h
+++ b/src/decompress/file_olefile.h
@@ -85,7 +85,7 @@ enum sector_type
     MINIFAT_SECTOR = 1
 };
 
-int32_t cli_readn(const uint8_t*& fd, int32_t& data_len, void* buff, int32_t count);
+int32_t cli_readn(const uint8_t*& fd, uint32_t& data_len, void* buff, int32_t count);
 
 struct FileProperty
 {
@@ -241,11 +241,13 @@ public:
     int32_t get_next_mini_fat_sector(int32_t sec_id);
     int32_t get_fat_offset(int32_t sec_id);
     int32_t get_mini_fat_offset(int32_t sec_id);
-    int32_t get_file_offset(const uint8_t*, int32_t data_len);
-    void get_file_data(char*, uint8_t*&, int32_t&);
+    int32_t get_file_offset(const uint8_t*, uint32_t data_len);
+    void get_file_data(char*, uint8_t*&, uint32_t&);
 
-    void decompression(const uint8_t* data, int32_t* data_len, uint8_t*& buffer,
-        uint32_t* buffer_ofset);
+    void decompression(const uint8_t* data, uint32_t& data_len, uint8_t*& buffer,
+        uint32_t& buffer_ofset);
+    uint32_t find_bytes_to_copy(uint32_t byte_offset, uint32_t data_len,
+                                   uint32_t stream_size, uint16_t sector_size);
 
     int search_nocase(const uint8_t* buffer, unsigned buffer_len) const
     {

diff --git a/src/decompress/file_oleheader.cc b/src/decompress/file_oleheader.cc
index 605e521514253040b179aad342366c6ed9fe53a7..8852d85256e054a3d6c7bdaf3f0df3aa96eba4f4 100644 (file)
--- a/src/decompress/file_oleheader.cc
+++ b/src/decompress/file_oleheader.cc
@@ -142,7 +142,7 @@ void OleHeader::set_minifat_cutoff(const uint8_t* buf)
     minifat_cutoff = (!byte_order_endian) ? LETOHL_UNALIGNED(buf) : BETOHL_UNALIGNED(buf);
 }
 
-int32_t OleHeader::get_minifat_cutoff()
+uint32_t OleHeader::get_minifat_cutoff()
 {
     return minifat_cutoff;
 }

diff --git a/src/decompress/file_oleheader.h b/src/decompress/file_oleheader.h
index e32a2c16022cfbeb45d093a40964b4b971647e86..7a560ac5dbc307123027c4648ff5aa81911ce4f3 100644 (file)
--- a/src/decompress/file_oleheader.h
+++ b/src/decompress/file_oleheader.h
@@ -73,7 +73,7 @@ public:
     void set_fat_sector_count(const uint8_t* buf);
     int32_t get_fat_sector_count();
     void set_minifat_cutoff(const uint8_t* buf);
-    int32_t get_minifat_cutoff();
+    uint32_t get_minifat_cutoff();
     void set_first_minifat(const uint8_t* buf);
     int32_t get_first_minifat();
     void set_minifat_count(const uint8_t* buf);
@@ -100,7 +100,7 @@ private:
     int32_t first_dir;
     int32_t difat_count;
     int32_t fat_sector_count;
-    int32_t minifat_cutoff;
+    uint32_t minifat_cutoff;
     int32_t first_minifat;
     int32_t minifat_count;
     int32_t first_difat;

diff --git a/src/decompress/test/file_olefile_test.cc b/src/decompress/test/file_olefile_test.cc
index a20965e98fb0d16542cdd5999374495b6d1b1d19..d3934ecf5879cdfdd21b94d44892e829a8498dfc 100644 (file)
--- a/src/decompress/test/file_olefile_test.cc
+++ b/src/decompress/test/file_olefile_test.cc
@@ -103,9 +103,10 @@ TEST(Olefile_ole, decompression_empty_data)
 {
     uint8_t* ole_data = nullptr;
     uint8_t* buf = nullptr;
-    int32_t len = 0;
+    uint32_t len = 0;
+    uint32_t buf_len;
     OleFile* olefile = new OleFile(nullptr, 0);
-    olefile->decompression(ole_data, &len, buf, nullptr);
+    olefile->decompression(ole_data, len, buf, buf_len);
     CHECK(buf == nullptr);
     delete olefile;
 }
@@ -114,9 +115,10 @@ TEST(Olefile_ole, decompression_invalid_data_1)
 {
     uint8_t ole_data[10] = { 0 };
     uint8_t* buf = nullptr;
-    int32_t len = 10;
+    uint32_t len = 10;
+    uint32_t buf_len;
     OleFile* olefile = new OleFile(nullptr, 0);
-    olefile->decompression(ole_data,&len, buf, nullptr);
+    olefile->decompression(ole_data,len, buf, buf_len);
     CHECK(buf == nullptr);
     delete olefile;
 }
@@ -125,10 +127,11 @@ TEST(Olefile_ole, decompression_invalid_chunk_header)
 {
     uint8_t ole_data[10] ={ 0 };
     uint8_t* buf = nullptr;
-    int32_t len = 10;
+    uint32_t len = 10;
+    uint32_t buf_len;
     ole_data[0] = 0x01;
     OleFile* olefile = new OleFile(nullptr, 0);
-    olefile->decompression(ole_data,&len, buf, nullptr);
+    olefile->decompression(ole_data, len, buf, buf_len);
     CHECK(buf == nullptr);
     delete olefile;
 }
@@ -137,10 +140,11 @@ TEST(Olefile_ole, decompression_flag_0)
 {
     uint8_t ole_data[10] ={ 0 };
     uint8_t* buf = nullptr;
-    int32_t len = 10;
+    uint32_t len = 10;
+    uint32_t buf_len;
     ole_data[0] = 0x01;
     OleFile* olefile = new OleFile(nullptr, 0);
-    olefile->decompression(ole_data,&len, buf, nullptr);
+    olefile->decompression(ole_data, len, buf, buf_len);
     CHECK(buf == nullptr);
     delete olefile;
 }
@@ -170,6 +174,15 @@ TEST(Olefile_ole, get_file_node_failure)
     CHECK(res == nullptr);
 }
 
+TEST(Olefile_ole, bytes_to_copy_test)
+{
+    OleFile* olefile = new OleFile(nullptr, 100);
+    uint32_t bytes_to_copy;
+    bytes_to_copy = olefile->find_bytes_to_copy(70, 50, 60, 64);
+    CHECK(bytes_to_copy == 10);
+    delete olefile;
+}
+
 TEST_GROUP(fat_mini_fat_list)
 {
 };