From 16fe6d3cb1835bd0cd852ad78f4b00c54e3fdcd1 Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Thu, 23 Feb 2017 10:32:12 -0600
Subject: SCI32: Store handle to VMD bitmap separately from ScreenItem

In at least RAMA, when using the pocket computer to view a mail
and then pressing the "return" button, the ScreenItem containing
the VMD will be destroyed before the VMD player is told to close
the video, resulting in a use-after-free trying to access the
bitmap ID through the deleted ScreenItem.
---
 engines/sci/graphics/video32.cpp | 10 ++++++----
 engines/sci/graphics/video32.h   |  5 +++++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index 8cd6fc7089..0e8ac8af41 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -614,7 +614,10 @@ VMDPlayer::IOStatus VMDPlayer::close() {
 		_bundledVmd = nullptr;
 	}
 
-	_segMan->freeBitmap(_screenItem->_celInfo.bitmap);
+	if (_bitmapId != NULL_REG) {
+		_segMan->freeBitmap(_bitmapId);
+		_bitmapId = NULL_REG;
+	}
 
 	if (!_planeIsOwned && _screenItem != nullptr) {
 		g_sci->_gfxFrameout->deleteScreenItem(*_screenItem);
@@ -738,8 +741,7 @@ VMDPlayer::EventFlags VMDPlayer::playUntilEvent(const EventFlags flags) {
 		const int16 scriptWidth = g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth;
 		const int16 scriptHeight = g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight;
 
-		reg_t bitmapId;
-		SciBitmap &vmdBitmap = *_segMan->allocateBitmap(&bitmapId, vmdRect.width(), vmdRect.height(), 255, 0, 0, screenWidth, screenHeight, 0, false, false);
+		SciBitmap &vmdBitmap = *_segMan->allocateBitmap(&_bitmapId, vmdRect.width(), vmdRect.height(), 255, 0, 0, screenWidth, screenHeight, 0, false, false);
 		vmdBitmap.getBuffer().fillRect(Common::Rect(vmdRect.width(), vmdRect.height()), 0);
 
 		if (screenWidth != scriptWidth || screenHeight != scriptHeight) {
@@ -747,7 +749,7 @@ VMDPlayer::EventFlags VMDPlayer::playUntilEvent(const EventFlags flags) {
 		}
 
 		CelInfo32 vmdCelInfo;
-		vmdCelInfo.bitmap = bitmapId;
+		vmdCelInfo.bitmap = _bitmapId;
 		_decoder->setSurfaceMemory(vmdBitmap.getPixels(), vmdBitmap.getWidth(), vmdBitmap.getHeight(), 1);
 
 		if (_planeIsOwned) {
diff --git a/engines/sci/graphics/video32.h b/engines/sci/graphics/video32.h
index fae5cafbbe..751604fd47 100644
--- a/engines/sci/graphics/video32.h
+++ b/engines/sci/graphics/video32.h
@@ -380,6 +380,11 @@ private:
 	 */
 	ScreenItem *_screenItem;
 
+	/**
+	 * The bitmap used to render the VMD.
+	 */
+	reg_t _bitmapId;
+
 	// TODO: planeIsOwned and priority are used in SCI3+ only
 
 	/**
-- 
cgit v1.2.3