OpenSkyscraper icon indicating copy to clipboard operation
OpenSkyscraper copied to clipboard

Palette generation is unsafe

Open daiplusplus opened this issue 4 years ago • 0 comments

I noticed this in SimTowerLoader.cpp, inside void SimTowerLoader::prepareBitmaps():

https://github.com/fabianschuiki/OpenSkyscraper/blob/a287b66cb03b84fc05d44f3c8f1543636379cf52/source/SimTowerLoader.cpp#L291-L299

Repeated as plaintext below for posterity - click to view
		//Assemble the palette
		Blob & palette = rawPalettes[0x83e8];
		char * dst = (bmp.data + 14 + 40);
		for (int i = 0; i < palette.length; i++) {
			(*dst++) = palette.data[i * 8 + 6];
			(*dst++) = palette.data[i * 8 + 4];
			(*dst++) = palette.data[i * 8 + 2];
			(*dst++) = 0;
		}

  1. The Blob instances in rawPalettes are initialized in preparePalettes().
  2. The char[] data array gets its length from Resource.length, which is always 2048 bytes for Palette resources (ResourceTypeId 0xFF03).
  3. In the above for loop, when i reaches 256 (in the range 0 to 2048) then it will dereference palette.data[ 256 * 8 + 6 ] => palette.data[2054] which is beyond the bounds of palette.data array.

If I understand the code correctly, it means that the SimTowerLoader program is always copying garbage bytes into the upper three-quarters of the DIB bitmap's palette area?

I note that I'm unable to build and run OpenSkyscraper so I can't run it under the debugger to find out why the out-of-bounds access seems to work.

daiplusplus avatar Aug 03 '21 08:08 daiplusplus