[Scummvm-git-logs] scummvm master -> d3da8a7367a5c8c6fb2cadc2c5aaf515d27165ed

[criezy]

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

Summary:
eff98b0c56 COMMON: Fix undefined behaviour in RandomSource constructor
d3da8a7367 GRAPHICS: Fix undefined behaviour in ManagedSurface blit on opaque target


Commit: eff98b0c563bfe64317bb79b044253f84330498c
    https://github.com/scummvm/scummvm/commit/eff98b0c563bfe64317bb79b044253f84330498c
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-10-31T21:46:22Z

Commit Message:
COMMON: Fix undefined behaviour in RandomSource constructor

The 'time.tm_year * 86400 * 366' line caused the result to overflow
what can be stored in an int; and signed int overflow is undefined
behaviour. The result goes into an unsigned int anyway, so now
all the intermediate computations are also done with unsigned int.
It still overflows (not on this line, but on the next one), but
that is fine as the standard guarantees that unsigned int overflow
wraps around.

Changed paths:
    common/random.cpp


diff --git a/common/random.cpp b/common/random.cpp
index d87881d883e..79009542e7c 100644
--- a/common/random.cpp
+++ b/common/random.cpp
@@ -36,10 +36,10 @@ RandomSource::RandomSource(const String &name) {
 #else
 	TimeDate time;
 	g_system->getTimeAndDate(time);
-	uint32 newSeed = time.tm_sec + time.tm_min * 60 + time.tm_hour * 3600;
-	newSeed += time.tm_mday * 86400 + time.tm_mon * 86400 * 31;
-	newSeed += time.tm_year * 86400 * 366;
-	newSeed = newSeed * 1000 + g_system->getMillis();
+	uint32 newSeed = time.tm_sec + time.tm_min * 60U + time.tm_hour * 3600U;
+	newSeed += time.tm_mday * 86400U + time.tm_mon * 86400U * 31U;
+	newSeed += time.tm_year * 86400U * 366U;
+	newSeed = newSeed * 1000U + g_system->getMillis();
 	setSeed(newSeed);
 #endif
 }


Commit: d3da8a7367a5c8c6fb2cadc2c5aaf515d27165ed
    https://github.com/scummvm/scummvm/commit/d3da8a7367a5c8c6fb2cadc2c5aaf515d27165ed
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-10-31T21:46:22Z

Commit Message:
GRAPHICS: Fix undefined behaviour in ManagedSurface blit on opaque target

The color components computation had intermediate results that could overflow
a signed int. So now the computation is done using unsigned int instead, which
prevents the overflow (since the max intermediate value is 255*255*257*257,
which fits in an unsigned int).

Note: I also considered adding an explicit cast to do the uint8 * uint8
operations using uint32, but decided not to as it is not required (there is no
overflow due to integer promotion) and makes the code more difficult to read.

Changed paths:
    graphics/managed_surface.cpp


diff --git a/graphics/managed_surface.cpp b/graphics/managed_surface.cpp
index 2a8cb25e542..a7ae3b1ad9e 100644
--- a/graphics/managed_surface.cpp
+++ b/graphics/managed_surface.cpp
@@ -401,9 +401,9 @@ void ManagedSurface::blitFromInner(const Surface &src, const Common::Rect &srcRe
 
 					if (aDest == 0xff) {
 						// Opaque target
-						rDest = (((rDest * (255 - aSrc) + rSrc * aSrc) * (257 * 257)) >> 24) & 0xff;
-						gDest = (((gDest * (255 - aSrc) + gSrc * aSrc) * (257 * 257)) >> 24) & 0xff;
-						bDest = (((bDest * (255 - aSrc) + bSrc * aSrc) * (257 * 257)) >> 24) & 0xff;
+						rDest = static_cast<uint8>((((rDest * (255U - aSrc) + rSrc * aSrc) * (257U * 257U)) >> 24) & 0xff);
+						gDest = static_cast<uint8>((((gDest * (255U - aSrc) + gSrc * aSrc) * (257U * 257U)) >> 24) & 0xff);
+						bDest = static_cast<uint8>((((bDest * (255U - aSrc) + bSrc * aSrc) * (257U * 257U)) >> 24) & 0xff);
 					} else {
 						// Translucent target
 						double sAlpha = (double)aSrc / 255.0;