Browse code

Fix bug in how ptrs to file data are used for computing Authenticode hash We used to get a pointer to file data without locking and for some samples this pointer would be invalidated by the time we used it. Now, we just store the offset for the sections that should be hashed as part of the Authenticode hash computation and get the file data pointer right before it's needed.

Andrew authored on 2018/09/06 07:50:59
Showing 3 changed files
... ...
@@ -2075,9 +2075,13 @@ int asn1_check_mscat(struct cl_engine *engine, fmap_t *map, size_t offset, unsig
2075 2075
 
2076 2076
     // Now that we know the hash algorithm, compute the authenticode hash
2077 2077
     // across the required regions of memory.
2078
-    // NOTE: section[i].ptr points to an already mapped region of memory
2079 2078
     for(int i = 0; i < nregions; i++) {
2080
-        cl_update_hash(ctx, regions[i].ptr, regions[i].size);
2079
+        const uint8_t *hptr; \
2080
+        if(!(hptr = fmap_need_off_once(map, regions[i].offset, regions[i].size))){
2081
+            return CL_VIRUS;
2082
+        }
2083
+
2084
+        cl_update_hash(ctx, hptr, regions[i].size);
2081 2085
     }
2082 2086
 
2083 2087
     cl_finish_hash(ctx, hash);
... ...
@@ -26,7 +26,7 @@
26 26
 #include "fmap.h"
27 27
 
28 28
 struct cli_mapped_region {
29
-    const unsigned char *ptr;
29
+    unsigned int offset;
30 30
     unsigned int size;
31 31
 };
32 32
 
... ...
@@ -5567,8 +5567,35 @@ int cli_checkfp_pe(cli_ctx *ctx, uint8_t *authsha1, stats_section_t *hashes, uin
5567 5567
         }
5568 5568
     }
5569 5569
 
5570
+    // TODO This likely isn't needed anymore, since we no longer compute
5571
+    // the authenticode hash like the 2008 spec doc says (sort sections
5572
+    // and use the section info to compute the hash)
5570 5573
     cli_qsort(exe_sections, nsections, sizeof(*exe_sections), sort_sects);
5571 5574
 
5575
+    /* Hash the sections */
5576
+    if (flags & CL_CHECKFP_PE_FLAG_STATS) {
5577
+
5578
+        for(i = 0; i < nsections; i++) {
5579
+            const uint8_t *hptr;
5580
+            void *md5ctx;
5581
+
5582
+            if(!exe_sections[i].rsz)
5583
+                continue;
5584
+
5585
+            if(!(hptr = fmap_need_off_once(map, exe_sections[i].raw, exe_sections[i].rsz))){
5586
+                free(exe_sections);
5587
+                return CL_EFORMAT;
5588
+            }
5589
+            md5ctx = cl_hash_init("md5");
5590
+            if (md5ctx) {
5591
+                cl_update_hash(md5ctx, (void *)hptr, exe_sections[i].rsz);
5592
+                cl_finish_hash(md5ctx, hashes->sections[i].md5);
5593
+            }
5594
+        }
5595
+    }
5596
+
5597
+    free(exe_sections);
5598
+
5572 5599
     if (flags & CL_CHECKFP_PE_FLAG_AUTHENTICODE) {
5573 5600
         /* Check to see if we have a security section. */
5574 5601
         if(!cli_hm_have_size(ctx->engine->hm_fp, CLI_HASH_SHA1, 2) && EC32(dirs[4].Size) < 8) {
... ...
@@ -5576,7 +5603,6 @@ int cli_checkfp_pe(cli_ctx *ctx, uint8_t *authsha1, stats_section_t *hashes, uin
5576 5576
                 /* If stats is enabled, continue parsing the sample */
5577 5577
                 flags ^= CL_CHECKFP_PE_FLAG_AUTHENTICODE;
5578 5578
             } else {
5579
-                free(exe_sections);
5580 5579
                 return CL_BREAK;
5581 5580
             }
5582 5581
         }
... ...
@@ -5590,7 +5616,6 @@ int cli_checkfp_pe(cli_ctx *ctx, uint8_t *authsha1, stats_section_t *hashes, uin
5590 5590
             if (flags & CL_CHECKFP_PE_FLAG_STATS) {
5591 5591
                 flags ^= CL_CHECKFP_PE_FLAG_AUTHENTICODE;
5592 5592
             } else {
5593
-                free(exe_sections);
5594 5593
                 return CL_EFORMAT;
5595 5594
             }
5596 5595
         }
... ...
@@ -5602,23 +5627,15 @@ int cli_checkfp_pe(cli_ctx *ctx, uint8_t *authsha1, stats_section_t *hashes, uin
5602 5602
     // data associated with each section.
5603 5603
     regions = (struct cli_mapped_region *) cli_calloc(nsections+4, sizeof(struct cli_mapped_region));
5604 5604
     if(!regions) {
5605
-        free(exe_sections);
5606 5605
         return CL_EMEM;
5607 5606
     }
5608 5607
     nregions = 0;
5609 5608
 
5610
-#define hash_chunk(where, _size) \
5609
+#define add_chunk_to_hash_list(_offset, _size) \
5611 5610
     do { \
5612
-        const uint8_t *hptr; \
5613
-        if(!(_size)) break; \
5614
-        if(!(hptr = fmap_need_off_once(map, where, _size))){ \
5615
-            free(exe_sections); \
5616
-            free(regions); \
5617
-            return CL_EFORMAT; \
5618
-        } \
5619 5611
         if (flags & CL_CHECKFP_PE_FLAG_AUTHENTICODE) { \
5620
-            regions[nregions].ptr = hptr; \
5621
-            regions[nregions].size = _size; \
5612
+            regions[nregions].offset = (_offset); \
5613
+            regions[nregions].size = (_size); \
5622 5614
             nregions++; \
5623 5615
         } \
5624 5616
     } while(0)
... ...
@@ -5627,7 +5644,7 @@ int cli_checkfp_pe(cli_ctx *ctx, uint8_t *authsha1, stats_section_t *hashes, uin
5627 5627
         /* MZ to checksum */
5628 5628
         at = 0;
5629 5629
         hlen = e_lfanew + sizeof(struct pe_image_file_hdr) + (pe_plus ? offsetof(struct pe_image_optional_hdr64, CheckSum) : offsetof(struct pe_image_optional_hdr32, CheckSum));
5630
-        hash_chunk(0, hlen);
5630
+        add_chunk_to_hash_list(0, hlen);
5631 5631
         at = hlen + 4;
5632 5632
 
5633 5633
         /* Checksum to security */
... ...
@@ -5635,7 +5652,7 @@ int cli_checkfp_pe(cli_ctx *ctx, uint8_t *authsha1, stats_section_t *hashes, uin
5635 5635
             hlen = offsetof(struct pe_image_optional_hdr64, DataDirectory[4]) - offsetof(struct pe_image_optional_hdr64, CheckSum) - 4;
5636 5636
         else
5637 5637
             hlen = offsetof(struct pe_image_optional_hdr32, DataDirectory[4]) - offsetof(struct pe_image_optional_hdr32, CheckSum) - 4;
5638
-        hash_chunk(at, hlen);
5638
+        add_chunk_to_hash_list(at, hlen);
5639 5639
         at += hlen + 8;
5640 5640
 
5641 5641
         if(at > hdr_size) {
... ...
@@ -5643,7 +5660,6 @@ int cli_checkfp_pe(cli_ctx *ctx, uint8_t *authsha1, stats_section_t *hashes, uin
5643 5643
                 flags ^= CL_CHECKFP_PE_FLAG_AUTHENTICODE;
5644 5644
                 break;
5645 5645
             } else {
5646
-                free(exe_sections);
5647 5646
                 free(regions);
5648 5647
                 return CL_EFORMAT;
5649 5648
             }
... ...
@@ -5651,42 +5667,17 @@ int cli_checkfp_pe(cli_ctx *ctx, uint8_t *authsha1, stats_section_t *hashes, uin
5651 5651
 
5652 5652
         /* Security to End of header */
5653 5653
         hlen = hdr_size - at;
5654
-        hash_chunk(at, hlen);
5654
+        add_chunk_to_hash_list(at, hlen);
5655 5655
         at += hlen;
5656 5656
         break;
5657 5657
     }
5658 5658
 
5659
-    /* Hash the sections */
5660
-    if (flags & CL_CHECKFP_PE_FLAG_STATS) {
5661
-
5662
-        for(i = 0; i < nsections; i++) {
5663
-            const uint8_t *hptr;
5664
-            void *md5ctx;
5665
-
5666
-            if(!exe_sections[i].rsz)
5667
-                continue;
5668
-
5669
-            if(!(hptr = fmap_need_off_once(map, exe_sections[i].raw, exe_sections[i].rsz))){
5670
-                free(exe_sections);
5671
-                free(regions);
5672
-                return CL_EFORMAT;
5673
-            }
5674
-            md5ctx = cl_hash_init("md5");
5675
-            if (md5ctx) {
5676
-                cl_update_hash(md5ctx, (void *)hptr, exe_sections[i].rsz);
5677
-                cl_finish_hash(md5ctx, hashes->sections[i].md5);
5678
-            }
5679
-        }
5680
-    }
5681
-
5682
-    free(exe_sections);
5683
-
5684 5659
     /* Finally, hash everything from the end of the header to the start of
5685 5660
      * the security section, which must be the last thing in a file
5686 5661
      */
5687 5662
     if (at < EC32(dirs[4].VirtualAddress)) {
5688 5663
         hlen = EC32(dirs[4].VirtualAddress)-at;
5689
-        hash_chunk(at, hlen);
5664
+        add_chunk_to_hash_list(at, hlen);
5690 5665
     }
5691 5666
 
5692 5667
     if (flags & CL_CHECKFP_PE_FLAG_AUTHENTICODE) {