Opened 18 years ago

Closed 18 years ago

#671 closed defect (invalid)

Corrupt File Crashes Game

Reported by: SF/jorpho Owned by: SF/ender
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Zak McKracken

Description

Edit the file 49.LFL in Zak256's file set, such that the byte at offset 34816 and all other bytes right to the end of the file are set to 00. (The resulting CRC32 should be 639AA283.)

Loading this fileset into version 0.3.0b (Win32) causes ScummVM to crash with an invalid page fault right after the title screen fades in. The precise message given by Windows 98 is

"SCUMMVM caused an invalid page fault in module SCUMMVM.EXE at 0177:0047ec09. Registers: EAX=00000800 CS=0177 EIP=0047ec09 EFLGS=00010216 EBX=0099a550 SS=017f ESP=025cfe28 EBP=025cfeb0 ECX=00000001 DS=017f ESI=00000000 FS=71b7 EDX=00000000 ES=017f EDI=c9311000 GS=0000 Bytes at CS:EIP: 8a 02 83 f0 80 25 ff 00 00 00 8b 55 1c 0f bf 04 Stack dump: 00000000 025cfe48 bff7b76c bff741fb bffc9490 bff713e2 00002000 865fb5bc 00991ec8 c9311000 00000000 00002000 00000000 025cfe98 c9311000 ffffffff"

ScummVM will continue to do things in the background until Mars is onscreen. The game will then cease to play, although F6 still brings up the options menu.

Ticket imported from: #667537. Ticket imported from: bugs/671.

Change History (4)

comment:1 by fingolfin, 18 years ago

We currently don't handle damaged data files well in any case and in every regard. You don't have to do it so complicated, even it's much easier to damage data files and get ScummVM to segfault immediatly :-)

I think we should put in many more checks for game data validity at some point, but right now many many many places contain code that makes assumptions about the data which only are valid if the data is not corrupt. Adding a lot of assert's would be a good start I guess.

Still, I am not sure if we want to keep this open as a bug, what do you think, Endy?

comment:2 by fingolfin, 18 years ago

Owner: set to SF/ender

comment:3 by SF/ender, 18 years ago

Resolution: invalid
Status: newclosed

comment:4 by SF/ender, 18 years ago

Hell no :)

The point is that corrupt data is IMPOSSIBLE to handle completely. If you chop or tweak a few bytes in the middle of room or sound resource data, or scripts, the first thing that will happen is a nasty segfault. There is no way to handle this without making every single decompression codec check every byte it reads, every length of compressed data that is expected... and (besides the 100x speed decrease such checking would ensure)
even then, how do you expect a game to continue if it's data is corrupt? :P

The best thing we could do is add some kind of signal handler and offer a nice "Sorry, go away" message - and that is far trickier under Windows than any other POSIX-complient system.

I do not believe adding in assertions is really of any use. The game will still crash, after all.. and you cannot easily add in asserts in the places crashes will most commonly occur - namely from decompression problems due to bad data.

Closing bug as invalid.

Note: See TracTickets for help on using tickets.