[Forensics-changes] [yara] 154/415: Clean up some string handling bugs, and fix a buffer overflow.

Hilko Bengen bengen at moszumanska.debian.org
Thu Apr 3 05:42:58 UTC 2014


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

bengen pushed a commit to branch debian
in repository yara.

commit cf31a19e90c5a17ed9741d5c57b32475632d3e24
Author: Victor M. Alvarez <plusvic at gmail.com>
Date:   Mon Apr 15 08:03:30 2013 +0000

    Clean up some string handling bugs, and fix a buffer overflow.
---
 libyara/ast.c     |   1 +
 libyara/grammar.c |  17 +++++++-
 libyara/grammar.y |  17 +++++++-
 libyara/lex.c     |   5 ++-
 libyara/lex.l     |   5 ++-
 libyara/proc.c    | 128 +++++++++++++++++++++++++++---------------------------
 6 files changed, 106 insertions(+), 67 deletions(-)

diff --git a/libyara/ast.c b/libyara/ast.c
index c14ca7f..47e9344 100644
--- a/libyara/ast.c
+++ b/libyara/ast.c
@@ -744,6 +744,7 @@ int new_variable(YARA_CONTEXT* context, char* identifier, TERM_VARIABLE** term)
     else
     {
         strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         result = ERROR_UNDEFINED_IDENTIFIER;
     }
     
diff --git a/libyara/grammar.c b/libyara/grammar.c
index 9e10058..a529e24 100644
--- a/libyara/grammar.c
+++ b/libyara/grammar.c
@@ -2775,6 +2775,7 @@ int reduce_rule_declaration(    yyscan_t yyscanner,
     if (context->last_result != ERROR_SUCCESS)
     {
         strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
     }
     else
     {
@@ -2786,6 +2787,7 @@ int reduce_rule_declaration(    yyscan_t yyscanner,
             {
                 context->last_result = ERROR_UNREFERENCED_STRING;
                 strncpy(context->last_error_extra_info, string->identifier, sizeof(context->last_error_extra_info));
+                context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
                 break;
             }
             
@@ -2817,12 +2819,14 @@ STRING* reduce_string_declaration(  yyscan_t yyscanner,
     
     if (context->last_result == ERROR_INVALID_REGULAR_EXPRESSION) 
     {
-        sprintf(tmp, "invalid regular expression in string \"%s\": %s", identifier, context->last_error_extra_info);
+        snprintf(tmp, sizeof(tmp), "invalid regular expression in string \"%s\": %s", identifier, context->last_error_extra_info);
         strncpy(context->last_error_extra_info, tmp, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
     }
     else if (context->last_result != ERROR_SUCCESS)
     {
         strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
     }
     
     yr_free(str);
@@ -2853,6 +2857,7 @@ STRING* reduce_strings( yyscan_t yyscanner,
     else
     {
         strncpy(context->last_error_extra_info, string->identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         context->last_result = ERROR_DUPLICATE_STRING_IDENTIFIER;
         return NULL;
     }   
@@ -2913,6 +2918,7 @@ META* reduce_metas( yyscan_t yyscanner,
     else
     {
         strncpy(context->last_error_extra_info, meta->identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         context->last_result = ERROR_DUPLICATE_META_IDENTIFIER;
         return NULL;
     }   
@@ -2945,6 +2951,7 @@ TAG* reduce_tags(   yyscan_t yyscanner,
     else
     {
         strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         context->last_result = ERROR_DUPLICATE_TAG_IDENTIFIER;
         return NULL;
     }
@@ -3014,6 +3021,7 @@ TERM* reduce_string(    yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
     }
     
@@ -3076,6 +3084,7 @@ TERM* reduce_string_at( yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
         else
         {
@@ -3101,6 +3110,7 @@ TERM* reduce_string_in_range(   yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
         else
         {
@@ -3125,6 +3135,7 @@ TERM* reduce_string_in_section_by_name( yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
         else
         {
@@ -3150,6 +3161,7 @@ TERM* reduce_string_count(  yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
     }
     
@@ -3171,6 +3183,7 @@ TERM* reduce_string_offset( yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
         else
         {
@@ -3271,12 +3284,14 @@ TERM* reduce_string_operation( yyscan_t yyscanner,
          else
          {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
             context->last_result = ERROR_INCORRECT_VARIABLE_TYPE;
          }
     }
     else
     {
         strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         context->last_result = ERROR_UNDEFINED_IDENTIFIER;
     }
     
diff --git a/libyara/grammar.y b/libyara/grammar.y
index 5e64e98..4dea10e 100644
--- a/libyara/grammar.y
+++ b/libyara/grammar.y
@@ -741,6 +741,7 @@ int reduce_rule_declaration(    yyscan_t yyscanner,
     if (context->last_result != ERROR_SUCCESS)
     {
         strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
     }
     else
     {
@@ -752,6 +753,7 @@ int reduce_rule_declaration(    yyscan_t yyscanner,
             {
                 context->last_result = ERROR_UNREFERENCED_STRING;
                 strncpy(context->last_error_extra_info, string->identifier, sizeof(context->last_error_extra_info));
+                context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
                 break;
             }
             
@@ -783,12 +785,14 @@ STRING* reduce_string_declaration(  yyscan_t yyscanner,
     
     if (context->last_result == ERROR_INVALID_REGULAR_EXPRESSION) 
     {
-        sprintf(tmp, "invalid regular expression in string \"%s\": %s", identifier, context->last_error_extra_info);
+        snprintf(tmp, sizeof(tmp), "invalid regular expression in string \"%s\": %s", identifier, context->last_error_extra_info);
         strncpy(context->last_error_extra_info, tmp, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
     }
     else if (context->last_result != ERROR_SUCCESS)
     {
         strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
     }
     
     yr_free(str);
@@ -819,6 +823,7 @@ STRING* reduce_strings( yyscan_t yyscanner,
     else
     {
         strncpy(context->last_error_extra_info, string->identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         context->last_result = ERROR_DUPLICATE_STRING_IDENTIFIER;
         return NULL;
     }   
@@ -879,6 +884,7 @@ META* reduce_metas( yyscan_t yyscanner,
     else
     {
         strncpy(context->last_error_extra_info, meta->identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         context->last_result = ERROR_DUPLICATE_META_IDENTIFIER;
         return NULL;
     }   
@@ -911,6 +917,7 @@ TAG* reduce_tags(   yyscan_t yyscanner,
     else
     {
         strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         context->last_result = ERROR_DUPLICATE_TAG_IDENTIFIER;
         return NULL;
     }
@@ -980,6 +987,7 @@ TERM* reduce_string(    yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
     }
     
@@ -1042,6 +1050,7 @@ TERM* reduce_string_at( yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
         else
         {
@@ -1067,6 +1076,7 @@ TERM* reduce_string_in_range(   yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
         else
         {
@@ -1091,6 +1101,7 @@ TERM* reduce_string_in_section_by_name( yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
         else
         {
@@ -1116,6 +1127,7 @@ TERM* reduce_string_count(  yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
     }
     
@@ -1137,6 +1149,7 @@ TERM* reduce_string_offset( yyscan_t yyscanner,
         if (context->last_result != ERROR_SUCCESS)
         {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         }
         else
         {
@@ -1237,12 +1250,14 @@ TERM* reduce_string_operation( yyscan_t yyscanner,
          else
          {
             strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+            context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
             context->last_result = ERROR_INCORRECT_VARIABLE_TYPE;
          }
     }
     else
     {
         strncpy(context->last_error_extra_info, identifier, sizeof(context->last_error_extra_info));
+        context->last_error_extra_info[sizeof(context->last_error_extra_info)-1] = 0;
         context->last_result = ERROR_UNDEFINED_IDENTIFIER;
     }
     
diff --git a/libyara/lex.c b/libyara/lex.c
index 67e001e..5847b54 100644
--- a/libyara/lex.c
+++ b/libyara/lex.c
@@ -1519,7 +1519,10 @@ YY_RULE_SETUP
 {
                                          int result;
 
-                                         sscanf( yytext + 2, "%x", &result );
+                                         if (sscanf( yytext + 2, "%x", &result ) != 1) {
+                                           yyerror(yyscanner, "Invalid escaped hex digit");
+                                           yyterminate();
+                                         }
                                          LEX_CHECK_SPACE_OK("X", yyextra->lex_buf_len, LEX_BUF_SIZE);
                                          *yyextra->lex_buf_ptr++ = result;
                                          yyextra->lex_buf_len++;
diff --git a/libyara/lex.l b/libyara/lex.l
index 8ad7553..99fee4d 100644
--- a/libyara/lex.l
+++ b/libyara/lex.l
@@ -344,7 +344,10 @@ $({letter}|{digit}|_)*               {
 <str>\\x{hexdigit}{2}                {
                                          int result;
 
-                                         sscanf( yytext + 2, "%x", &result );
+                                         if (sscanf( yytext + 2, "%x", &result ) != 1) {
+                                           yyerror(yyscanner, "Invalid escaped hex digit");
+                                           yyterminate();
+                                         }
                                          LEX_CHECK_SPACE_OK("X", yyextra->lex_buf_len, LEX_BUF_SIZE);
                                          *yyextra->lex_buf_ptr++ = result;
                                          yyextra->lex_buf_len++;
diff --git a/libyara/proc.c b/libyara/proc.c
index 9bb060c..86402dd 100644
--- a/libyara/proc.c
+++ b/libyara/proc.c
@@ -26,58 +26,58 @@ int get_process_memory(int pid, MEMORY_BLOCK** first_block)
 {
     PVOID address;
     SIZE_T read;
-    
+
     unsigned char* data;
-    
+
     SYSTEM_INFO si;
     MEMORY_BASIC_INFORMATION mbi;
 
     MEMORY_BLOCK* new_block;
     MEMORY_BLOCK* current_block = NULL;
-    
+
     TOKEN_PRIVILEGES tokenPriv;
-    LUID luidDebug; 
+    LUID luidDebug;
     HANDLE hProcess;
     HANDLE hToken;
-    
-    if( OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES, &hToken) && 
-        LookupPrivilegeValue(NULL, SE_DEBUG_NAME, &luidDebug)) 
-    {  
-            tokenPriv.PrivilegeCount = 1; 
-            tokenPriv.Privileges[0].Luid = luidDebug; 
-            tokenPriv.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED; 
-
-            AdjustTokenPrivileges(hToken, FALSE, &tokenPriv, sizeof(tokenPriv), NULL, NULL); 
-    }  
- 
+
+    if( OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES, &hToken) &&
+        LookupPrivilegeValue(NULL, SE_DEBUG_NAME, &luidDebug))
+    {
+            tokenPriv.PrivilegeCount = 1;
+            tokenPriv.Privileges[0].Luid = luidDebug;
+            tokenPriv.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED;
+
+            AdjustTokenPrivileges(hToken, FALSE, &tokenPriv, sizeof(tokenPriv), NULL, NULL);
+    }
+
     hProcess = OpenProcess(PROCESS_VM_READ | PROCESS_QUERY_INFORMATION, FALSE, pid);
-    
+
     *first_block = NULL;
-    
+
     if (hProcess == NULL)
     {
         return ERROR_COULD_NOT_ATTACH_TO_PROCESS;
     }
-    
+
     GetSystemInfo(&si);
-    
+
     address = si.lpMinimumApplicationAddress;
 
-    while (address < si.lpMaximumApplicationAddress) 
+    while (address < si.lpMaximumApplicationAddress)
     {
          if (VirtualQueryEx(hProcess, address, &mbi, sizeof(mbi)) != 0)
-         {         
+         {
              if (mbi.State == MEM_COMMIT && mbi.Protect != PAGE_NOACCESS)
-             {    
+             {
                  data = (unsigned char*) yr_malloc(mbi.RegionSize);
-             
+
                  if (data == NULL)
                      return ERROR_INSUFICIENT_MEMORY;
-             
+
                  if (ReadProcessMemory(hProcess, address, data, mbi.RegionSize, &read))
-                 {                
+                 {
                      new_block = (MEMORY_BLOCK*) yr_malloc(sizeof(MEMORY_BLOCK));
-                 
+
                      if (new_block == NULL)
                      {
                          yr_free(data);
@@ -102,21 +102,21 @@ int get_process_memory(int pid, MEMORY_BLOCK** first_block)
                      yr_free(data);
                  }
              }
-             
+
              address = (PVOID)((DWORD) mbi.BaseAddress + mbi.RegionSize);
-         }    
+         }
      }
-     
+
      return ERROR_SUCCESS;
 }
 
 #else
 
 #include <fcntl.h>
-#include <unistd.h> 
+#include <unistd.h>
 #include <sys/types.h>
 #include <sys/ptrace.h>
-#include <sys/wait.h>  
+#include <sys/wait.h>
 
 #include "mem.h"
 #include "proc.h"
@@ -137,20 +137,20 @@ int get_process_memory(pid_t pid, MEMORY_BLOCK** first_block)
 {
     task_t task;
     kern_return_t kr;
-    
+
     vm_size_t size = 0;
-    vm_address_t address = 0;  
+    vm_address_t address = 0;
     vm_region_basic_info_data_64_t info;
     mach_msg_type_number_t info_count;
     mach_port_t object;
-    
+
     unsigned char* data;
-    
+
     MEMORY_BLOCK* new_block;
     MEMORY_BLOCK* current_block = NULL;
-    
+
     *first_block = NULL;
-    
+
     if ((kr = task_for_pid(mach_task_self(), pid, &task)) != KERN_SUCCESS)
     {
         return ERROR_COULD_NOT_ATTACH_TO_PROCESS;
@@ -159,20 +159,20 @@ int get_process_memory(pid_t pid, MEMORY_BLOCK** first_block)
     do {
 
          info_count = VM_REGION_BASIC_INFO_COUNT_64;
-         
+
          kr = vm_region_64(task, &address, &size, VM_REGION_BASIC_INFO, (vm_region_info_t) &info, &info_count, &object);
-                  
-         if (kr == KERN_SUCCESS) 
-         {         
+
+         if (kr == KERN_SUCCESS)
+         {
              data = (unsigned char*) yr_malloc(size);
-             
+
              if (data == NULL)
                  return ERROR_INSUFICIENT_MEMORY;
-             
+
              if (vm_read_overwrite(task, address, size, (vm_address_t) data, &size) == KERN_SUCCESS)
-             {                
+             {
                  new_block = (MEMORY_BLOCK*) yr_malloc(sizeof(MEMORY_BLOCK));
-                 
+
                  if (new_block == NULL)
                  {
                      yr_free(data);
@@ -192,18 +192,18 @@ int get_process_memory(pid_t pid, MEMORY_BLOCK** first_block)
 
                  current_block = new_block;
              }
-             
+
              address += size;
-         } 
+         }
+
 
-         
      } while (kr != KERN_INVALID_ADDRESS);
-     
-     if (task != MACH_PORT_NULL) 
+
+     if (task != MACH_PORT_NULL)
      {
           mach_port_deallocate(mach_task_self(), task);
      }
-     
+
      return ERROR_SUCCESS;
 }
 
@@ -216,16 +216,16 @@ int get_process_memory(pid_t pid, MEMORY_BLOCK** first_block)
     char buffer[256];
     unsigned char* data;
     size_t begin, end, length;
-    
+
     MEMORY_BLOCK* new_block;
     MEMORY_BLOCK* current_block = NULL;
-    
+
     *first_block = NULL;
 
     sprintf(buffer, "/proc/%u/maps", pid);
 
     FILE* maps = fopen(buffer, "r");
-    
+
     if (maps == NULL)
     {
         return ERROR_COULD_NOT_ATTACH_TO_PROCESS;
@@ -245,41 +245,43 @@ int get_process_memory(pid_t pid, MEMORY_BLOCK** first_block)
     {
         return ERROR_COULD_NOT_ATTACH_TO_PROCESS;
     }
-    
+
     wait(NULL);
 
     while (fgets(buffer, sizeof(buffer), maps) != NULL)
     {
-        sscanf(buffer, "%lx-%lx", &begin, &end);
+        if (sscanf(buffer, "%lx-%lx", &begin, &end) != 2 || end < begin) {
+          continue;
+        }
 
         length = end - begin;
 
         data = yr_malloc(length);
-        
+
         if (data == NULL)
             return ERROR_INSUFICIENT_MEMORY;
 
         if (pread(mem, data, length, begin) != -1)
         {
             new_block = (MEMORY_BLOCK*) yr_malloc(sizeof(MEMORY_BLOCK));
-            
+
             if (new_block == NULL)
             {
                 yr_free(data);
                 return ERROR_INSUFICIENT_MEMORY;
             }
-            
+
             if (*first_block == NULL)
                 *first_block = new_block;
-            
+
             new_block->base = begin;
             new_block->size = length;
             new_block->data = data;
             new_block->next = NULL;
-            
+
             if (current_block != NULL)
                 current_block->next = new_block;
-            
+
             current_block = new_block;
         }
     }
@@ -288,7 +290,7 @@ int get_process_memory(pid_t pid, MEMORY_BLOCK** first_block)
 
     close(mem);
     fclose(maps);
- 
+
     return ERROR_SUCCESS;
 }
 

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



More information about the forensics-changes mailing list