aboutsummaryrefslogtreecommitdiff
path: root/engines
diff options
context:
space:
mode:
authorColin Snover2017-06-17 14:17:16 -0500
committerColin Snover2017-06-17 14:35:42 -0500
commit832cd25ef1a5cd2dc9cb8062f043fb402dab6ed7 (patch)
tree83ed5cc4dbd8243ade09912f247f4db8505fafe5 /engines
parent0d63d2a7adbe4381d945228a9da4471c9409a8f1 (diff)
downloadscummvm-rg350-832cd25ef1a5cd2dc9cb8062f043fb402dab6ed7.tar.gz
scummvm-rg350-832cd25ef1a5cd2dc9cb8062f043fb402dab6ed7.tar.bz2
scummvm-rg350-832cd25ef1a5cd2dc9cb8062f043fb402dab6ed7.zip
SCI32: Avoid out-of-bounds read of pixel data in kIsOnMe
Fixes Trac#9761, Trac#9844, Trac#9850, Trac#9851.
Diffstat (limited to 'engines')
-rw-r--r--engines/sci/graphics/frameout.cpp18
1 files changed, 18 insertions, 0 deletions
diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index b4c842dd4a..c969f91c71 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -1229,6 +1229,24 @@ bool GfxFrameout::isOnMe(const ScreenItem &screenItem, const Plane &plane, const
scaledPosition.y = scaledPosition.y * 128 / screenItem._scale.y;
}
+ // TODO/HACK: When clicking at the very bottom edge of a scaled cel, it
+ // is possible that the calculated `scaledPosition` ends up one pixel
+ // outside of the bounds of the cel. It is not known yet whether this is
+ // a bug that also existed in SSCI (and so garbage memory would be read
+ // there), or if there is actually an error in our scaling of
+ // `ScreenItem::_screenRect` and/or `scaledPosition`. For now, just do
+ // an extra bounds check and return so games don't crash when a user
+ // clicks an unlucky point. Later, double-check the disassembly and
+ // either confirm this is a suitable fix (because SSCI just read bad
+ // memory) or fix the actual broken thing and remove this workaround.
+ if (scaledPosition.x < 0 ||
+ scaledPosition.y < 0 ||
+ scaledPosition.x >= celObj._width ||
+ scaledPosition.y >= celObj._height) {
+
+ return false;
+ }
+
uint8 pixel = celObj.readPixel(scaledPosition.x, scaledPosition.y, mirrorX);
return pixel != celObj._skipColor;
}