[Scummvm-git-logs] scummvm master -> 164498f204ba60cf9089af16ccd6febbbd0d2d8d

[elasota]

This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
164498f204 MTROPOLIS: Fix out-of-range read when loading 8-bit mToon assets


Commit: 164498f204ba60cf9089af16ccd6febbbd0d2d8d
    https://github.com/scummvm/scummvm/commit/164498f204ba60cf9089af16ccd6febbbd0d2d8d
Author: elasota (1137273+elasota at users.noreply.github.com)
Date: 2024-01-02T22:34:06-05:00

Commit Message:
MTROPOLIS: Fix out-of-range read when loading 8-bit mToon assets

Changed paths:
    engines/mtropolis/assets.cpp
    engines/mtropolis/assets.h


diff --git a/engines/mtropolis/assets.cpp b/engines/mtropolis/assets.cpp
index eb0e8bf3d57..64c28574580 100644
--- a/engines/mtropolis/assets.cpp
+++ b/engines/mtropolis/assets.cpp
@@ -314,6 +314,29 @@ bool CachedMToon::decompressMToonRLE(const RleFrame &frame, const Common::Array<
 	return true;
 }
 
+template<class TDest, class TSrc>
+void CachedMToon::checkedMemCpy(Common::Array<TDest> &dest, size_t destIndex, const Common::Array<TSrc> &src, size_t srcIndex, size_t sizeBytes) {
+	if (sizeBytes == 0)
+		return;
+
+	size_t destSize = dest.size() * sizeof(TDest);
+	size_t srcSize = src.size() * sizeof(TSrc);
+
+	if (destIndex > dest.size() || srcIndex > src.size())
+		error("Out-of-range data copy offset while loading mToon");
+
+	size_t srcPos = srcIndex * sizeof(TSrc);
+	size_t destPos = destIndex * sizeof(TDest);
+
+	size_t srcAvail = srcSize - srcPos;
+	size_t destAvail = destSize - destPos;
+
+	if (srcAvail < sizeBytes || destAvail < sizeBytes)
+		error("Out-of-range data copy end while loading mToon");
+
+	memcpy(&dest[destIndex], &src[srcIndex], sizeBytes);
+}
+
 void CachedMToon::decompressRLEFrameToImage(size_t frameIndex, Graphics::ManagedSurface &surface) {
 	assert(surface.format == _rleOptimizedFormat);
 
@@ -348,6 +371,9 @@ void CachedMToon::loadRLEFrames(const Common::Array<uint8> &data) {
 
 		size_t baseOffset = frameDef.dataOffset;
 
+		if (frameDef.compressedSize < 20)
+			error("Invalid compressed data size");
+
 		uint32 headerInts[5];
 		for (size_t hi = 0; hi < 5; hi++) {
 			uint32 unpacked = 0;
@@ -373,15 +399,15 @@ void CachedMToon::loadRLEFrames(const Common::Array<uint8> &data) {
 		uint32 frameDataSize = headerInts[4];
 
 		if (frameDataSize > 0) {
+			// frameDataSize is sometimes set to frameDef.compressedSize but sometimes contains garbage,
+			// so we need to ignore it and derive size from the frameDef instead.
 			if (bpp == 8) {
-				rleFrame.data8.resize(frameDataSize);
-				memcpy(&rleFrame.data8[0], &data[baseOffset + 20], frameDataSize);
+				rleFrame.data8.resize(frameDef.compressedSize - 20);
+				checkedMemCpy(rleFrame.data8, 0, data, baseOffset + 20, frameDef.compressedSize - 20);
 			} else if (bpp == 16) {
-				// In RLE16, frameDataSize is sometimes set to frameDef.compressedSize but sometimes contains garbage,
-				// so we need to ignore it and derive size from the frameDef instead.
 				uint32 numDWords = (frameDef.compressedSize - 20) / 2;
 				rleFrame.data16.resize(numDWords);
-				memcpy(&rleFrame.data16[0], &data[baseOffset + 20], static_cast<size_t>(numDWords) * 2u);
+				checkedMemCpy(rleFrame.data16, 0, data, baseOffset + 20, static_cast<size_t>(numDWords) * 2u);
 
 				uint16 *i16 = &rleFrame.data16[0];
 				if (_metadata->imageFormat == MToonMetadata::kImageFormatWindows) {
diff --git a/engines/mtropolis/assets.h b/engines/mtropolis/assets.h
index f440199c2f2..1b809e30cf4 100644
--- a/engines/mtropolis/assets.h
+++ b/engines/mtropolis/assets.h
@@ -146,6 +146,9 @@ private:
 	template<class TNumber, uint32 TLiteralMask, uint32 TTransparentRowSkipMask>
 	static bool decompressMToonRLE(const RleFrame &frame, const Common::Array<TNumber> &coefsArray, Graphics::ManagedSurface &surface, bool isBottomUp, bool isKeyFrame, uint hackFlags);
 
+	template<class TDest, class TSrc>
+	static void checkedMemCpy(Common::Array<TDest> &dest, size_t destIndex, const Common::Array<TSrc> &src, size_t srcIndex, size_t sizeBytes);
+
 	Common::Array<RleFrame> _rleData;
 	bool _isRLETemporalCompressed;