[Forensics-changes] [yara] 08/160: PE module: Fix unchecked access to version info buffers

[Hilko Bengen]

This is an automated email from the git hooks/post-receive script.

bengen pushed a commit to annotated tag v3.4.0
in repository yara.

commit 088fc84b07dca304d6eeff34f3994178fac00ec6
Author: Moritz Kroll <moritz.kroll at avira.com>
Date:   Thu Feb 12 00:03:27 2015 +0100

    PE module: Fix unchecked access to version info buffers
    
    Fixes crash (at least on Windows) for
    https://www.virustotal.com/en/file/a906103c7b972e7adf8ea9c736ca44373949480576d7fc6042c495c47e9bbff5/analysis/1423694573/
    and adds additional safety for other cases like unterminated keys
---
 libyara/include/yara/strutils.h |  5 +++--
 libyara/modules/pe.c            | 38 +++++++++++++++-----------------------
 libyara/strutils.c              |  8 +++++---
 3 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/libyara/include/yara/strutils.h b/libyara/include/yara/strutils.h
index 3e38bc4..62dd497 100644
--- a/libyara/include/yara/strutils.h
+++ b/libyara/include/yara/strutils.h
@@ -59,8 +59,9 @@ void* memmem(
 #endif
 
 
-int strlen_w(
-    const char* w_str);
+int strnlen_w(
+    const char* w_str,
+    size_t maxbytes);
 
 
 int strcmp_w(
diff --git a/libyara/modules/pe.c b/libyara/modules/pe.c
index 2bc1e21..2b26de5 100644
--- a/libyara/modules/pe.c
+++ b/libyara/modules/pe.c
@@ -619,10 +619,7 @@ void pe_parse_version_info(
 
   version_info = (PVERSION_INFO) (pe->data + version_info_offset);
 
-  if (!struct_fits_in_pe(pe, version_info, VERSION_INFO))
-    return;
-
-  if (!fits_in_pe(pe, version_info, sizeof("VS_VERSION_INFO")))
+  if (!fits_in_pe(pe, version_info->Key, sizeof("VS_VERSION_INFO")))
     return;
 
   if (strcmp_w(version_info->Key, "VS_VERSION_INFO") != 0)
@@ -630,13 +627,8 @@ void pe_parse_version_info(
 
   string_file_info = ADD_OFFSET(version_info, sizeof(VERSION_INFO) + 86);
 
-  if (!struct_fits_in_pe(pe, string_file_info, VERSION_INFO))
-    return;
-
-  if (!fits_in_pe(pe, string_file_info, sizeof("StringFileInfo")))
-    return;
-
-  while(strcmp_w(string_file_info->Key, "StringFileInfo") == 0)
+  while(fits_in_pe(pe, string_file_info->Key, sizeof("StringFileInfo")) &&
+      strcmp_w(string_file_info->Key, "StringFileInfo") == 0)
   {
     PVERSION_INFO string_table = ADD_OFFSET(
         string_file_info,
@@ -646,37 +638,37 @@ void pe_parse_version_info(
         string_file_info,
         string_file_info->Length);
 
-    while (string_table < string_file_info)
+    while (struct_fits_in_pe(pe, string_table, VERSION_INFO) &&
+        string_table->Length != 0 &&
+        string_table < string_file_info)
     {
       PVERSION_INFO string = ADD_OFFSET(
           string_table,
-          sizeof(VERSION_INFO) + 2 * (strlen_w(string_table->Key) + 1));
+          sizeof(VERSION_INFO) + 2 * (strnlen_w(string_table->Key,
+              available_space(pe, string_table->Key)) + 1));
 
       string_table = ADD_OFFSET(
           string_table,
           string_table->Length);
 
       while (struct_fits_in_pe(pe, string, VERSION_INFO) &&
+             string->Length != 0 &&
              string < string_table)
       {
         char* string_value = (char*) ADD_OFFSET(
             string,
-            sizeof(VERSION_INFO) + 2 * (strlen_w(string->Key) + 1));
+            sizeof(VERSION_INFO) + 2 * (strnlen_w(string->Key,
+                available_space(pe, string->Key)) + 1));
 
-        strlcpy_w(key, string->Key, sizeof(key));
-        strlcpy_w(value, string_value, sizeof(value));
+        strlcpy_w(key, string->Key, min(sizeof(key),
+            available_space(pe, string->Key)));
+        strlcpy_w(value, string_value, min(sizeof(value),
+            available_space(pe, string_value)));
 
         set_string(value, pe->object, "version_info[%s]", key);
 
-        if (string->Length == 0)
-          break;
-
         string = ADD_OFFSET(string, string->Length);
       }
-
-      if (!struct_fits_in_pe(pe, string_table, VERSION_INFO) ||
-          string_table->Length == 0)
-        break;
     }
   }
 }
diff --git a/libyara/strutils.c b/libyara/strutils.c
index 9255370..eff9f5a 100644
--- a/libyara/strutils.c
+++ b/libyara/strutils.c
@@ -153,15 +153,17 @@ size_t strlcat(
 #endif
 
 
-int strlen_w(
-    const char* w_str)
+int strnlen_w(
+    const char* w_str,
+    size_t maxbytes)
 {
   int len = 0;
 
-  while (*w_str != 0)
+  while (maxbytes >= 2 && (w_str[0] || w_str[1]))
   {
     w_str += 2;
     len += 1;
+    maxbytes -= 2;
   }
 
   return len;

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/forensics/yara.git