michi_cc wrote:

Patch seems good, I'm wondering why the code would use "y != 0 || x < info->width" instead of "y != 0 && x < info->width" for the while condition though. Anything I'm missing on the RLE encoding here?

This comment was imported from FlySpray: https://bugs.openttd.org/task/4746#comment10231

michi_cc wrote:

Using && instead of || would fix a part of the exploit, but would return the wrong return value, so extra y == 0 checks are needed.

The broken code is in trunk since the TGP merge at r5946 which means 0.5.0 and later.

This comment was imported from FlySpray: https://bugs.openttd.org/task/4746#comment10236

monoid wrote:

I've taken the opportunity to fix/clean up the BMP RLE decoding routines further in this new patch.

It successfully decodes the attached 4-bit RLE BMP image, whereas 1.1.2 fails to decode it. I closely followed the 'specs' at http://www.fileformat.info/format/bmp/egff.htm , and paid attention to the remarks shown at http://trac.webkit.org/browser/trunk/Source/WebCore/platform/image-decoders/bmp/BMPImageReader.cpp?rev=77429# L503 (it is actually images with these overlong uncompressed rows that 1.1.2 currently chokes on).

It is easier codewise to deal with an uninverted y-coordinate, and simply invert it when using it to get the pixel write pointer position.

Attachments

• fixbmprle2.diff (5.31 KiB)
• FIFE_6-16-rle.bmp (9.09 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/4746#comment10241

monoid wrote:

Er, that's actually an 8-bit image (with 16 colours) >_>.

Either way, it doesn't currently load in 1.1.2, due to it not handling overlong uncompressed rows as mentioned.

Also, should the BMP-internal file buffering stuff be removed, and replaced with the generic FioReadXXX() system?

This comment was imported from FlySpray: https://bugs.openttd.org/task/4746#comment10242

monoid wrote:

Fixed the patch, added some necessary y-coordinate checks.

Attachments

• fixbmprle3.diff (5.44 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/4746#comment10243

monoid wrote:

This additional patch fixes up the error reporting when a heightmap image fails to load - currently the error condition is ignored.

Attachments

• fixheightmaperrors.diff (6.38 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/4746#comment10274

michi_cc closed the ticket.

Reason for closing: Implemented

In r22871 and r22872.

This comment was imported from FlySpray: https://bugs.openttd.org/task/4746