From 859c0efb01b901c5ee669585fa99b608c60c4d94 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 20 Sep 2026 11:56:05 -0400 Subject: [PATCH] [MG_Remote] validate packed readback bounds before copying GPU replies --- MobileGL/MG_Remote/Client/EmitTables.cpp | 11 ++- .../MG_Remote/Client/TextureReadbackEmit.inc | 86 +++++++++++++------ 2 files changed, 71 insertions(+), 26 deletions(-) diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index 6d4341b3..476d9689 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -902,6 +902,8 @@ namespace MobileGL::MG_Remote::Client { } } +#include "TextureReadbackEmit.inc" + void EmitReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, void* pixels) { ClientSession& session = RequireSession("ReadPixels"); @@ -968,6 +970,13 @@ namespace MobileGL::MG_Remote::Client { if (MG_State::pGLContext != nullptr) { pack = MG_State::pGLContext->GetPixelStoreParameters(/*isUnpack=*/false); } + ReadbackLayout layout; + const SizeT pboOffset = reinterpret_cast(pixels); + if (!CheckedReadbackLayout(width, height, 1, bytesPerPixel, pack, false, layout) || + (pbo && (pboOffset > pbo->GetSize() || layout.End > pbo->GetSize() - pboOffset))) { + RecordReadbackRangeError("ReadPixels"); + return; + } Int32 status = 0; Uint64 replySize = 0; @@ -1014,8 +1023,6 @@ namespace MobileGL::MG_Remote::Client { pack); } -#include "TextureReadbackEmit.inc" - void EmitPresent() { ClientSession& session = RequireSession("Present"); BeforeReadOnlyVerb(); diff --git a/MobileGL/MG_Remote/Client/TextureReadbackEmit.inc b/MobileGL/MG_Remote/Client/TextureReadbackEmit.inc index f3062d14..61d1f518 100644 --- a/MobileGL/MG_Remote/Client/TextureReadbackEmit.inc +++ b/MobileGL/MG_Remote/Client/TextureReadbackEmit.inc @@ -1,9 +1,49 @@ // Included in EmitTables.cpp's client-only anonymous namespace. // The server returns owned tight pixels. Only the client interprets PACK state // and application pointers; PBO rows are ordinary buffer uploads after the reply. + struct ReadbackLayout { + Uint64 RowBytes = 0, RowStride = 0, ImageStride = 0, Start = 0, End = 0, Tight = 0; + }; + + Bool CheckedReadbackLayout(Uint64 width, Uint64 height, Uint64 depth, Uint64 bpp, + const PixelStoreParameters& pack, Bool imagePack, ReadbackLayout& out) { + constexpr Uint64 limit = std::numeric_limits::max(); + auto mul = [&](Uint64 a, Uint64 b, Uint64& result) { + if (a && b > limit / a) return false; + result = a * b; return true; + }; + auto add = [&](Uint64 a, Uint64 b, Uint64& result) { + if (a > limit || b > limit - a) return false; + result = a + b; return true; + }; + if (!width || !height || !depth || !bpp) return false; + const Uint64 align = std::max(pack.Alignment, 1); + Uint64 rawRow = 0, rounded = 0, skipRows = 0, skipPixels = 0, skipImages = 0; + Uint64 lastImage = 0, lastRow = 0; + if (!mul(width, bpp, out.RowBytes) || + !mul(pack.RowLength > 0 ? pack.RowLength : width, bpp, rawRow) || + !add(rawRow, align - 1, rounded) || + !mul(rounded / align, align, out.RowStride) || + !mul(imagePack && pack.ImageHeight > 0 ? pack.ImageHeight : height, out.RowStride, out.ImageStride) || + !mul(pack.SkipRows, out.RowStride, skipRows) || !mul(pack.SkipPixels, bpp, skipPixels) || + !mul(imagePack ? pack.SkipImages : 0, out.ImageStride, skipImages) || + !add(skipRows, skipPixels, out.Start) || !add(out.Start, skipImages, out.Start) || + !mul(depth - 1, out.ImageStride, lastImage) || !mul(height - 1, out.RowStride, lastRow) || + !add(out.Start, lastImage, out.End) || !add(out.End, lastRow, out.End) || + !add(out.End, out.RowBytes, out.End) || !mul(out.RowBytes, height, out.Tight) || + !mul(out.Tight, depth, out.Tight)) return false; + return true; + } + + void RecordReadbackRangeError(const char* caller) { + MG_State::pGLContext->RecordError(ErrorCode::InvalidOperation, + MakeUnique("MG_Remote/Client", caller, + "Pixel packing exceeds the destination or its addressable range.")); + } + void EmitGetTextureImage(const SharedPtr& texture, TextureUploadTarget uploadTarget, GLint level, GLenum format, - GLenum type, GLsizei, void* pixels) { + GLenum type, GLsizei bufSize, void* pixels) { if (!texture || texture->GetStorageType() != TextureStorageType::Mipmap) return; auto* mipmap = static_cast(texture.get()); const Bool wholeCube = texture->GetTarget() == TextureTarget::TextureCubeMap && @@ -11,13 +51,26 @@ const auto firstTarget = wholeCube ? TextureUploadTarget::CubeMapPositiveX : uploadTarget; const IntVec3 size = mipmap->GetMipmapTexelSize(firstTarget, level); if (size.x() <= 0 || size.y() <= 0 || size.z() <= 0) return; + const Uint64 bpp = ReadbackBytesPerPixel(format, type); + const Uint64 depth = wholeCube ? 6 : static_cast(size.z()); + const auto pack = MG_State::pGLContext->GetPixelStoreParameters(false); + const auto target = texture->GetTarget(); + const Bool imagePack = wholeCube || target == TextureTarget::Texture3D || + target == TextureTarget::Texture2DArray || target == TextureTarget::TextureCubeMapArray; + const auto pbo = MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelPack).GetBoundObject(); + const SizeT pboOffset = reinterpret_cast(pixels); + ReadbackLayout layout; + if (!CheckedReadbackLayout(size.x(), size.y(), depth, bpp, pack, imagePack, layout) || + (bufSize >= 0 && layout.End > static_cast(bufSize)) || + (pbo && (pboOffset > pbo->GetSize() || layout.End > pbo->GetSize() - pboOffset))) { + RecordReadbackRangeError("GetTextureImage"); + return; + } + const Uint64 tight = layout.Tight; ClientSession& session = RequireSession("GetTextureImage"); BeforeReadOnlyVerb(); // A by-name texture need not occur in any currently bound sampler. MG_Pipe::MGPipeTextureEmitterInstance().DrainTextureSubData(*MG_State::pGLContext); - const Uint64 bpp = ReadbackBytesPerPixel(format, type); - const Uint64 depth = wholeCube ? 6 : static_cast(size.z()); - const Uint64 tight = static_cast(size.x()) * size.y() * depth * bpp; MG_Pipe::MGPReadbackInfo info{}; info.Res = MG_Pipe::MGPipeSlots().FindByLifetimeId(MG_Pipe::MGPipeKind::Texture, texture->GetLifetimeId()); @@ -40,35 +93,20 @@ RequireReadbackReplyComplete(status, received, info.DstSize); offset += info.DstSize; } - const auto pack = MG_State::pGLContext->GetPixelStoreParameters(false); ApplyReadbackByteSwap(bytes.data(), tight, type, pack); - const Uint64 rowBytes = static_cast(size.x()) * bpp; - const Uint64 alignment = std::max(pack.Alignment, 1); - const Uint64 rowPixels = pack.RowLength > 0 ? pack.RowLength : size.x(); - const Uint64 rowStride = (rowPixels * bpp + alignment - 1) / alignment * alignment; - const auto target = texture->GetTarget(); - const Bool imagePack = wholeCube || target == TextureTarget::Texture3D || - target == TextureTarget::Texture2DArray || target == TextureTarget::TextureCubeMapArray; - const Uint64 imageRows = imagePack && pack.ImageHeight > 0 ? pack.ImageHeight : size.y(); - const Uint64 imageStride = imageRows * rowStride; - const Uint64 start = static_cast(pack.SkipRows) * rowStride + - static_cast(pack.SkipPixels) * bpp + - (imagePack ? static_cast(pack.SkipImages) * imageStride : 0); - const auto& pbo = MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelPack).GetBoundObject(); for (Uint64 z = 0; z < depth; ++z) { for (Int y = 0; y < size.y(); ++y) { - const Uint8* source = bytes.data() + (z * size.y() + y) * rowBytes; - const Uint64 offset = start + z * imageStride + y * rowStride; - if (pbo) pbo->UploadSubData({source, static_cast(rowBytes)}, - reinterpret_cast(pixels) + offset); - else if (pixels) Memcpy(static_cast(pixels) + offset, source, rowBytes); + Uint8* source = bytes.data() + (z * size.y() + y) * layout.RowBytes; + const Uint64 offset = layout.Start + z * layout.ImageStride + y * layout.RowStride; + if (pbo) pbo->UploadSubData({source, static_cast(layout.RowBytes)}, pboOffset + offset); + else if (pixels) Memcpy(static_cast(pixels) + offset, source, layout.RowBytes); } } } void EmitGetTexImage(GLenum target, GLint level, GLenum format, GLenum type, void* pixels) { const auto textureTarget = MG_Util::ConvertGLEnumToTextureTarget(target); - const auto& unit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit()); + auto& unit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit()); const auto& texture = unit.GetBindingSlot(textureTarget).GetBoundObject(); EmitGetTextureImage(texture, MG_Util::ConvertGLEnumToTextureUploadTarget(target), level, format, type, -1, pixels);