thirdparty/scummvm
2
0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

GRAPHICS: Fix undefined behaviour in ManagedSurface blit on opaque target

Browse source 
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.
This commit is contained in:
Thierry Crozat 2022-10-31 21:27:16 +00:00
parent eff98b0c56
commit d3da8a7367
1 changed files with 3 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

6
graphics/managed_surface.cpp
View file

@ -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;