[syslinux] [PATCH 4/5] installers: fix a possible buffer overflow when looking for LDLINUX_MAGIC

[Shao Miller]

On 2/24/2016 08:02, Pete Batard via Syslinux wrote:
> If the ldlinux being processed is garbage, the search for 
> LDLINUX_MAGIC will overflow its buffer - fix that.
> I did encounter this issue in Rufus as, due to notorious 
> incompatibilities between different versions of ldlinux.sys and the 
> com32's residing on an ISO, we download a version specific ldlinux.sys 
> from our server... which may get trashed if the user sits behind one 
> of these corporate firewalls that modifies the download payload and 
> replaces it with something like "You are not authorized to download 
> this file"...

$0.02:
- Casting to a uintptr_t is ugly (and not C89, not that Syslinux cares 
about that)
- The boot_image + boot_image_len calculation is loop invariant, so some 
kind of boot_image_end or wpe pointer before the loop might be nicer.  
Unfortunately, the 8-bit boot_image_len (bytes) versus the 32-bit wp 
stride complicates things

Maybe something like:

   const uint32_t * wpe = (const uint32_t *) boot_image + boot_len / 
sizeof *wpe;

where the 'for' check could then: ... && wp < wpe

We all know that it wouldn't actually bite us, but technically, if 
boot_image's memory's size isn't evenly divisible by 32 bits, the wp++ 
can land us in a position where a read access would yield undefined 
behaviour.  Maybe boot_image memory always will be a multiple of 4; I 
don't know.

With the proposed patch's uintptr_t stuff, if the magic isn't found:

              * <------- boot_image_len dictates the final byte
            * <-------- wp is less than boot_image_len
00001111222233XXYYYY <- XX are out-of-bound bytes
                 * <---- Where wp is when the loop breaks
               * <------ As far as any kind of pointer should point
             * <-------- When the loop should break, as 33XX can't 
contain the magic

Fingers crossed that my math is working.

--
- Synthetel
http://Shao Miller