MYST ME (gog version) Crash

[weirdzod]

Version: The version included in Gog MYST ME.
SO: Windows 7 64 Home Premium
Game Language: Spanish

How to reproduce:
1- Start a new game
2- go to the "zepelin" and turn on the switch
3- go to the library
5- in the map, select the zepelin
4- open the secret door to the elevator
5- go to the elevator
6- go to the "watts" signal
Crash!

As my English is not very good I have recorded a video to show it.

​https://www.youtube.com/watch?v=qVve_iNHEt4

[digitall]

Have watched video. weirdzod's English is pretty good, but to clarify slightly.

The replication is to start MYST, switch on the Marker Switch for the Rocketship, then use the Painting in the Library to do the Tower Rotation to point the Tower at the Rocketship. Open the Bookshelf Door using the other Painting, then go up in the Tower Elevator. Climb the Ladder for the Key i.e. Solution for this which will show the 59V plaque for the Rocketship Puzzle. Click on the Plaque for the close up causes the crash.

The video also shows various images localised to Spanish text including the Tower Rotation, Elevator Label and probably the close up images. I would suspect that there is a image size difference which causes an out of bounds image blit or clip in the Mohawk engine which causes this.

I don't have the Spanish version, but since this is relatively inexpensive on GOG, I will buy a copy shortly and run this under valgrind or similar to try to get a backtrace.

[digitall]

Replicated with the latest ScummVM git master i.e. f0f02c4a3f1be13058c2334bae694312a315a030 under Linux amd64. This crashes out at the same point with:
free(): corrupted unsorted chunks
Aborted

Will run gdb / valgrind and try to get a backtrace on the cause.

[digitall]

gdb backtrace...

free(): corrupted unsorted chunks

Thread 1 "scummvm" received signal SIGABRT, Aborted.
0x00007ffff4316f50 in raise () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff4316f50 in raise () from /lib64/libc.so.6
#1  0x00007ffff4318b1d in abort () from /lib64/libc.so.6
#2  0x00007ffff435d280 in __libc_message () from /lib64/libc.so.6
#3  0x00007ffff4364b38 in malloc_printerr () from /lib64/libc.so.6
#4  0x00007ffff4365e90 in _int_free () from /lib64/libc.so.6
#5  0x00005555557caa9b in Graphics::Surface::free (this=0x555556597e20)
    at graphics/surface.cpp:80
#6  0x00005555557ae832 in Image::JPEGDecoder::destroy (this=0x555556597e10)
    at image/jpeg.cpp:59
#7  0x00005555557ae7b3 in Image::JPEGDecoder::~JPEGDecoder (
    this=0x555556597e10, __in_chrg=<optimized out>) at image/jpeg.cpp:51
#8  0x00005555557ae7f0 in Image::JPEGDecoder::~JPEGDecoder (
    this=0x555556597e10, __in_chrg=<optimized out>) at image/jpeg.cpp:52
#9  0x000055555578a556 in Image::PICTDecoder::decodeCompressedQuickTime (
    this=0x7ffffffb7970, stream=...) at image/pict.cpp:585
#10 0x0000555555788195 in Image::PICTDecoder::on_compressedQuickTime (
    this=0x7ffffffb7970, stream=...) at image/pict.cpp:188
#11 0x00005555557885f8 in Image::PICTDecoder::loadStream (this=0x7ffffffb7970, 
    stream=...) at image/pict.cpp:253
#12 0x0000555555618cda in Mohawk::MystGraphics::decodeImage (
    this=0x555556545b20, id=4464) at engines/mohawk/myst_graphics.cpp:131
#13 0x0000555555690015 in Mohawk::GraphicsManager::findImage (
    this=0x555556545b20, id=4464) at engines/mohawk/graphics.cpp:93
#14 0x0000555555619497 in Mohawk::MystGraphics::copyImageSectionToBackBuffer (
    this=0x555556545b20, image=4464, src=..., dest=...)
    at engines/mohawk/myst_graphics.cpp:263
#15 0x00005555556103c5 in Mohawk::MystAreaImageSwitch::drawDataToScreen (
    this=0x555555f559e0) at engines/mohawk/myst_areas.cpp:429
#16 0x0000555555615dff in Mohawk::MystCard::drawResourceImages (
    this=0x555555fe06a0) at engines/mohawk/myst_card.cpp:443
#17 0x0000555555614088 in Mohawk::MystCard::enter (this=0x555555fe06a0)
    at engines/mohawk/myst_card.cpp:58
#18 0x000055555560ae90 in Mohawk::MohawkEngine_Myst::changeToCard (
    this=0x5555565535d0, card=4461, transition=Mohawk::kTransitionDissolve)
    at engines/mohawk/myst.cpp:770
#19 0x000055555561ec30 in Mohawk::MystScriptParser::o_goToDestForward (
    this=0x55555653aca0, var=0, args=...)
    at engines/mohawk/myst_scripts.cpp:416
#20 0x0000555555621e0e in Common::Functor2Mem<unsigned short, Common::Array<unsigned short> const&, void, Mohawk::MystScriptParser>::operator() (
    this=0x555555fe0740, v1=0, v2=...) at ./common/func.h:507
#21 0x000055555561ddcc in Mohawk::MystScriptParser::runOpcode (
    this=0x55555653aca0, op=6, var=0, args=...)
    at engines/mohawk/myst_scripts.cpp:195
#22 0x000055555560e7fd in Mohawk::MystArea::handleMouseUp (this=0x555555fde8c0)
    at engines/mohawk/myst_areas.cpp:98
#23 0x00005555556159ca in Mohawk::MystCard::updateResourcesForInput (
    this=0x555555f51790, mouse=..., mouseClicked=false, mouseMoved=false)
    at engines/mohawk/myst_card.cpp:384
#24 0x0000555555609edd in Mohawk::MohawkEngine_Myst::doFrame (
    this=0x5555565535d0) at engines/mohawk/myst.cpp:557
#25 0x00005555556096a1 in Mohawk::MohawkEngine_Myst::run (this=0x5555565535d0)
    at engines/mohawk/myst.cpp:399
#26 0x00005555555c2398 in runGame (plugin=0x555555b38d30, system=..., 
    edebuglevels=...) at base/main.cpp:264
#27 0x00005555555c366b in scummvm_main (argc=1, argv=0x7fffffffdc88)
    at base/main.cpp:532
#28 0x00005555555c059c in main (argc=1, argv=0x7fffffffdc88)
    at backends/platform/sdl/posix/posix-main.cpp:45

[digitall]

Crash is prevented when running under Valgrind, but there is an invalid access on that card i.e. closeup of the solution for Rocketship Puzzle. See below:

==620== Invalid write of size 2
==620==    at 0x4C3150B: memcpy@GLIBC_2.2.5 (vg_replace_strmem.c:1021)
==620==    by 0x33E4E6: Image::PICTDecoder::decodeCompressedQuickTime(Common::SeekableReadStream&) (pict.cpp:582)
==620==    by 0x33C194: Image::PICTDecoder::on_compressedQuickTime(Common::SeekableReadStream&) (pict.cpp:188)
==620==    by 0x33C5F7: Image::PICTDecoder::loadStream(Common::SeekableReadStream&) (pict.cpp:253)
==620==    by 0x1CCCD9: Mohawk::MystGraphics::decodeImage(unsigned short) (myst_graphics.cpp:131)
==620==    by 0x244014: Mohawk::GraphicsManager::findImage(unsigned short) (graphics.cpp:93)
==620==    by 0x1CD496: Mohawk::MystGraphics::copyImageSectionToBackBuffer(unsigned short, Common::Rect, Common::Rect) (myst_graphics.cpp:263)
==620==    by 0x1C43C4: Mohawk::MystAreaImageSwitch::drawDataToScreen() (myst_areas.cpp:429)
==620==    by 0x1C9DFE: Mohawk::MystCard::drawResourceImages() (myst_card.cpp:443)
==620==    by 0x1C8087: Mohawk::MystCard::enter() (myst_card.cpp:58)
==620==    by 0x1BEE8F: Mohawk::MohawkEngine_Myst::changeToCard(unsigned short, Mohawk::TransitionType) (myst.cpp:770)
==620==    by 0x1D2C2F: Mohawk::MystScriptParser::o_goToDestForward(unsigned short, Common::Array<unsigned short> const&) (myst_scripts.cpp:416)
==620==  Address 0xd7eb018 is 0 bytes after a block of size 32,136 alloc'd
==620==    at 0x4C2EF25: calloc (vg_replace_malloc.c:711)
==620==    by 0x37EA34: Graphics::Surface::create(unsigned short, unsigned short, Graphics::PixelFormat const&) (surface.cpp:74)
==620==    by 0x33E449: Image::PICTDecoder::decodeCompressedQuickTime(Common::SeekableReadStream&) (pict.cpp:578)
==620==    by 0x33C194: Image::PICTDecoder::on_compressedQuickTime(Common::SeekableReadStream&) (pict.cpp:188)
==620==    by 0x33C5F7: Image::PICTDecoder::loadStream(Common::SeekableReadStream&) (pict.cpp:253)
==620==    by 0x1CCCD9: Mohawk::MystGraphics::decodeImage(unsigned short) (myst_graphics.cpp:131)
==620==    by 0x244014: Mohawk::GraphicsManager::findImage(unsigned short) (graphics.cpp:93)
==620==    by 0x1CD496: Mohawk::MystGraphics::copyImageSectionToBackBuffer(unsigned short, Common::Rect, Common::Rect) (myst_graphics.cpp:263)
==620==    by 0x1C43C4: Mohawk::MystAreaImageSwitch::drawDataToScreen() (myst_areas.cpp:429)
==620==    by 0x1C9DFE: Mohawk::MystCard::drawResourceImages() (myst_card.cpp:443)
==620==    by 0x1C8087: Mohawk::MystCard::enter() (myst_card.cpp:58)
==620==    by 0x1BEE8F: Mohawk::MohawkEngine_Myst::changeToCard(unsigned short, Mohawk::TransitionType) (myst.cpp:770)
==620== 

This is either a bug in our PICT decoder or a malformed PICT in the resource file (though the decoder should do more sanity checks to avoid bad accesses in that case).

[digitall]

Ah. I think I see the root of the problem at image/pict.cpp line 582 i.e.

	for (uint16 y = 0; y < surface->h; y++)
		memcpy(_outputSurface->getBasePtr(0 + xOffset, y + yOffset), surface->getBasePtr(0, y), surface->w * surface->format.bytesPerPixel);

The size of _outputSurface is derived from the _imageRect size which is not related to the size of the decoded surface so this can result in an out of bounds copy.

@bgK: Can you take a look at adding some sanity limits for the copy from the decoded surface to the _outputSurface to prevent any out of bounds writes?

[bgK]

Thanks for your report. This bug was fixed in commit 360e1e97c9c5, included in ScummVM daily builds with version 2.1.0git-3687 and above.