Subject: bug#20264: closed (Re: bug#20264: [PATCH] fix: w32_executable_type() causes a segmentation fault)

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 06 Apr 2015 10:31:02 +0000

Your bug report

#20264: [PATCH] fix: w32_executable_type() causes a segmentation fault

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
20264: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20264
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#20264: [PATCH] fix: w32_executable_type() causes a segmentation fault
From: Eli Zaretskii <[email-masked]>
To: Koichi Arakawa <[email-masked]>
Cc: [email-masked]
Date: Mon, 06 Apr 2015 13:30:11 +0300
List-Id: <debbugs-submit.debbugs.gnu.org>

> Date: Mon, 06 Apr 2015 18:48:11 +0900
>  (東京 (標準時))
> Cc: [email-masked]
> From: Koichi Arakawa <[email-masked]>
> 
> I apologize insufficient research. The *illegal* dllname actually
> points to another section. So the previous patch is wrong and it
> should be as follows.

Thanks, I pushed it.

Subject: [PATCH] fix: w32_executable_type() causes a segmentation fault
From: Koichi Arakawa <[email-masked]>
To: [email-masked]
Date: Mon, 06 Apr 2015 12:23:23 +0900(東京 (標準時))
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi folks,

On Windows platform, w32_executable_type() in src/w32proc.c scans
'dllname' in an EXE file. But there are some strange EXE files that
'dllname' points to an illegal address, for example, Microsoft's Excel
(excel.exe) and PowerPoint (POWEPNT.EXE). w32_executable_type() causes
a segmentation fault for those files.

objdump in binutils seems to know those illegal pointers and discard
them (pe_print_idata() in bfd/peXXigen.c).

In the following patch, 'dllname' is checked whether it points to the
valid section's address space and discarded when it's invalid.

Regards,
Koichi Arakawa

diff --git a/src/ChangeLog b/src/ChangeLog
index 1c3f933..a49fdf4 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2015-04-06  Koichi Arakawa  <[email-masked]>
+
+	* w32proc.c (w32_executable_type): Check whether 'dllname' points
+	to the section's address space.
+
 2015-04-04  Jan Djärv  <[email-masked]>
 
 	* xselect.c (x_reply_selection_request)
diff --git a/src/w32proc.c b/src/w32proc.c
index 7d982f8..d3d9405 100644
--- a/src/w32proc.c
+++ b/src/w32proc.c
@@ -1618,16 +1618,23 @@ w32_executable_type (char * filename,
                 data_dir[IMAGE_DIRECTORY_ENTRY_IMPORT];
               IMAGE_IMPORT_DESCRIPTOR * imports;
               IMAGE_SECTION_HEADER * section;
+              char * base;
+              DWORD_PTR real_size;
 
               section = rva_to_section (import_dir.VirtualAddress, nt_header);
               imports = RVA_TO_PTR (import_dir.VirtualAddress, section,
                                     executable);
+              base = RVA_TO_PTR (section->VirtualAddress, section, executable);
+              real_size = max (section->SizeOfRawData, section->Misc.VirtualSize);
 
               for ( ; imports->Name; imports++)
                 {
                   char * dllname = RVA_TO_PTR (imports->Name, section,
                                                executable);
 
+                  if (imports->Name < base || dllname >= base + real_size)
+                    break;
+
                   /* The exact name of the cygwin dll has changed with
                      various releases, but hopefully this will be reasonably
                      future proof.  */