On Sat, Oct 06, 2018 at 09:46:36PM +0300, Konstantin Belousov wrote:
> On Sat, Oct 06, 2018 at 09:21:04PM +0300, Konstantin Belousov wrote:
> > On Sat, Oct 06, 2018 at 08:35:26PM +0300, l...@lena.kiev.ua wrote:
> > > > Insufficient validation was performed in the ELF header parser, and 
> > > > malformed
> > > > or otherwise invalid ELF binaries were not rejected as they should be.
> > > 
> > > What is invalid in the /usr/local/share/google-earth/googleearth-bin
> > > binary of the port google-earth-7.1.5.1557,3 ?
> > > 
> > > FreeBSD 11.2-RELEASE-p4 Sep 27 GENERIC i386, the binary:
> > > https://drive.google.com/file/d/1SgHk8ijSp2F9UcQGlx44psT832TdIEL0/view
> > > 
> > > ~ $ googleearth
> > > Invalid PT_INTERP
> > > exec: ./googleearth-bin: Exec format error
> > > ~ $ readelf --program-headers 
> > > /usr/local/share/google-earth/googleearth-bin
> > > 
> > > Elf file type is EXEC (Executable file)
> > > Entry point 0x8048650
> > > There are 8 program headers, starting at offset 52
> > > 
> > > Program Headers:
> > >   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
> > >   PHDR           0x000034 0x08048034 0x08048034 0x00100 0x00100 R E 0x4
> > >   INTERP         0x000134 0x08048134 0x08048134 0x00011 0x00011 R   0x1
> > >       [Requesting program interpreter: /lib/ld-linux.so.2]
> > As you see, the file delcares that file/memory length of the interpreter
> > name' segment is 0x11 == 16 decimal. But the string does not end on
> > byte 16, which is not NUL.  We tighten the checks and do require that
> > PT_INTERP string is valid by checking that it is NUL-terminated at the
> > offset declared by the size.
> As emaste pointed out, I am off by one, i.e. replace 16 by 17 in the text
> above.

But we might be somewhat nicer in this case.  Try the following.

diff --git a/sys/kern/imgact_elf.c b/sys/kern/imgact_elf.c
index f4302d46665..88f8a1ed2fa 100644
--- a/sys/kern/imgact_elf.c
+++ b/sys/kern/imgact_elf.c
@@ -872,9 +872,23 @@ __CONCAT(exec_, __elfN(imgact))(struct image_params *imgp)
                                interp = __DECONST(char *, imgp->image_header) +
                                    phdr[i].p_offset;
                                if (interp[interp_name_len - 1] != '\0') {
-                                       uprintf("Invalid PT_INTERP\n");
-                                       error = ENOEXEC;
-                                       goto ret;
+                                       /*
+                                        * ELF specification requires
+                                        * that PT_INTERP contained
+                                        * NUL-terminated string.  If
+                                        * it is not, try to fix the
+                                        * path and still execute the
+                                        * binary.
+                                        */
+                                       VOP_UNLOCK(imgp->vp, 0);
+                                       interp_buf = malloc(interp_name_len + 1,
+                                           M_TEMP, M_WAITOK);
+                                       vn_lock(imgp->vp, LK_EXCLUSIVE |
+                                           LK_RETRY);
+                                       memcpy(interp_buf, interp,
+                                           interp_name_len);
+                                       interp_buf[interp_name_len] = '\0';
+                                       interp = interp_buf;
                                }
                        }
                        break;
_______________________________________________
freebsd-security@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-security
To unsubscribe, send any mail to "freebsd-security-unsubscr...@freebsd.org"

Reply via email to