SCUMM: Add some bounds-checking to o5_stringOps() (bug #15884)#6586
SCUMM: Add some bounds-checking to o5_stringOps() (bug #15884)#6586bluegr merged 1 commit intoscummvm:masterfrom
Conversation
This is primarily to fix out-of-bounds reads in the Fate of Atlantis copy protection script, but it doesn't hurt to do it always. And we may as well stop out-of-bounds writes while we're at it.
|
Thanks! Bounds checking on strings is a step towards the right direction :) |
|
I just noticed that VGA Loom seems to write outside a string resource when starting the game:
I wonder what that string resource is used for. |
|
It looks like the resource manager allocates 2 extra bytes for each resource? (See |
|
I've relaxed the bounds checking so that reading/writing to the "safety area" is allowed, though a warning is still printed. |
2 participants
As described in https://bugs.scummvm.org/ticket/15884 there appears to be a script bug in the Fate of Atlantis copy protection. This is basically what the room-81-204 script does:
So it verifies that the mouse X position is within the copy protection stones, but doesn't care about the Y position. As you move the mouse to the top of the screen, negative indexes become first possible, and then inevitable.
In this particular script, we should return 48 (ASCII for "0") to indicate a non-clickable part of the stones. But anything that isn't 49-56 goes to the same "do nothing" default case so returning 0 should work just as well, and now we have well defined behavior across all games.
I don't know of any game that tries to write outside a string, but it doesn't hurt to catch that case as well.