From c30e78b9074fed5c510cf0781b8dcc2200179e0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Lend=C3=ADnez?= <52505803+JaviLendi@users.noreply.github.com> Date: Sun, 10 May 2026 17:17:40 +0200 Subject: [PATCH 1/2] Potential fix for code scanning alert no. 227: Multiplication result converted to larger type Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- src/Screenshot.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Screenshot.cpp b/src/Screenshot.cpp index ef35ba8fdf1..a7135d3bcf0 100644 --- a/src/Screenshot.cpp +++ b/src/Screenshot.cpp @@ -814,7 +814,7 @@ static void PaintOverlayLayered(HWND hwnd, ScreenshotOverlayData* data) { bmiTemp.bmiHeader.biPlanes = 1; bmiTemp.bmiHeader.biBitCount = 32; bmiTemp.bmiHeader.biCompression = BI_RGB; - DWORD* tempPixels = (DWORD*)malloc(w * h * 4); + DWORD* tempPixels = (DWORD*)malloc((size_t)w * (size_t)h * sizeof(DWORD)); GetDIBits(hdcTemp, hbmTemp, 0, h, tempPixels, &bmiTemp, DIB_RGB_COLORS); // Copy thumbnail and label regions with full opacity into the DIB From a92b49f10f0d31f16bfb740f68766cd8757601fd Mon Sep 17 00:00:00 2001 From: "[JaviLendi]" Date: Sun, 10 May 2026 17:57:13 +0200 Subject: [PATCH 2/2] Improve memory safety and WCHAR casting Replace unsafe char->WCHAR casts with str::CastToWCHAR and fix ToUtf8Temp usage (EbookDoc, EngineImages, StrconvUtil). Add bounds and overflow checks for allocations and early returns to prevent integer/size_t overflows and invalid allocations (Screenshot, SumatraDialogs). Fix format specifiers and pointer formatting in logging (SearchAndDDE, SumatraPDF). Use the original view mode string when building DDE commands to avoid unnecessary wide-string conversion (SumatraStartup). Adjust TGA serialization to use size_t casts for safe size calculations and allocation (TgaReader). These changes address potential UB, allocation overflows and improve robustness. --- src/EbookDoc.cpp | 4 ++-- src/EngineImages.cpp | 3 ++- src/Screenshot.cpp | 18 +++++++++++++++++- src/SearchAndDDE.cpp | 2 +- src/SumatraDialogs.cpp | 8 +++++++- src/SumatraPDF.cpp | 4 ++-- src/SumatraStartup.cpp | 3 +-- src/utils/StrconvUtil.cpp | 7 +++---- src/utils/TgaReader.cpp | 4 ++-- 9 files changed, 37 insertions(+), 16 deletions(-) diff --git a/src/EbookDoc.cpp b/src/EbookDoc.cpp index c0e3b5bfa97..6ac3b9b3965 100644 --- a/src/EbookDoc.cpp +++ b/src/EbookDoc.cpp @@ -93,13 +93,13 @@ static TempStr DecodeTextToUtf8Temp(const char* s, bool isXML = false) { if (str::StartsWith(s, UTF16BE_BOM)) { // convert from utf16 big endian to utf16 s += 2; - int n = str::Leni((WCHAR*)s); + WCHAR* ws = str::CastToWCHAR(s); + int n = str::Leni(ws); char* tmp = (char*)s; for (int i = 0; i < n; i++) { int idx = i * 2; std::swap(tmp[idx], tmp[idx + 1]); } - WCHAR* ws = str::CastToWCHAR(s); return ToUtf8Temp(ws); } uint codePage = isXML ? GetCodepageFromPI(s) : CP_ACP; diff --git a/src/EngineImages.cpp b/src/EngineImages.cpp index 91a7047f646..2e7e824ec9c 100644 --- a/src/EngineImages.cpp +++ b/src/EngineImages.cpp @@ -1459,7 +1459,8 @@ static void GetBitmapExifProperties(Bitmap* bmp, StrVec& keyValOut) { } // Unicode else if (memcmp(item->value, "UNICODE\0", 8) == 0) { - val = ToUtf8Temp((WCHAR*)commentData, commentLen / 2); + WCHAR* ws = str::CastToWCHAR(commentData); + val = ToUtf8Temp(ws, commentLen / 2); if (val && !str::IsEmpty(val)) { AddProp(keyValOut, kPropUserComment, val); } diff --git a/src/Screenshot.cpp b/src/Screenshot.cpp index a7135d3bcf0..ca77a705681 100644 --- a/src/Screenshot.cpp +++ b/src/Screenshot.cpp @@ -295,7 +295,23 @@ static void FixRoundedCorners(HBITMAP hbm, int w, int h, COLORREF bgColor, int r InitBitmapInfo32(bmi, w, h); HDC hdc = GetDC(nullptr); - DWORD* pixels = (DWORD*)malloc(w * h * 4); + if (w <= 0 || h <= 0) { + ReleaseDC(nullptr, hdc); + return; + } + size_t sw = (size_t)w; + size_t sh = (size_t)h; + if (sw > (SIZE_MAX / sh)) { + ReleaseDC(nullptr, hdc); + return; + } + size_t pixelCount = sw * sh; + if (pixelCount > (SIZE_MAX / sizeof(DWORD))) { + ReleaseDC(nullptr, hdc); + return; + } + size_t allocSize = pixelCount * sizeof(DWORD); + DWORD* pixels = (DWORD*)malloc(allocSize); if (!pixels) { ReleaseDC(nullptr, hdc); return; diff --git a/src/SearchAndDDE.cpp b/src/SearchAndDDE.cpp index 39c3dcaaf9f..58b1d54275e 100644 --- a/src/SearchAndDDE.cpp +++ b/src/SearchAndDDE.cpp @@ -1241,7 +1241,7 @@ LRESULT OnDDERequest(HWND hwnd, WPARAM wp, LPARAM lp) { // we handle those break; default: - logf("OnDDERequest: invalid fmt '%s'\n", (int)fmt); + logf("OnDDERequest: invalid fmt '%u'\n", (unsigned)fmt); return 0; } ATOM a = HIWORD(lp); diff --git a/src/SumatraDialogs.cpp b/src/SumatraDialogs.cpp index d08794a2d35..a3cd81dac14 100644 --- a/src/SumatraDialogs.cpp +++ b/src/SumatraDialogs.cpp @@ -1221,7 +1221,13 @@ static void PaintColorArea(HDC hdc, RECT* rc) { } // rows must be DWORD-aligned; each pixel is 3 bytes (BGR) int stride = (w * 3 + 3) & ~3; - u8* bits = (u8*)malloc(stride * h); + size_t strideSz = (size_t)stride; + size_t hSz = (size_t)h; + if (strideSz > SIZE_MAX / hSz) { + return; + } + size_t bitsSize = strideSz * hSz; + u8* bits = (u8*)malloc(bitsSize); if (!bits) { return; } diff --git a/src/SumatraPDF.cpp b/src/SumatraPDF.cpp index 1b40d76f954..c2a5018eeb8 100644 --- a/src/SumatraPDF.cpp +++ b/src/SumatraPDF.cpp @@ -3342,7 +3342,7 @@ void CloseTab(WindowTab* tab, bool quitIfLast) { // are other windows, else the Frequently Read page is displayed void CloseCurrentTab(MainWindow* win, bool quitIfLast) { WindowTab* tab = win->CurrentTab(); - logf("CloseCurrentTab: tab: 0x%p win: 0x%p, hwndFrame: 0x%x, quitIfLast: %d\n", tab, win, win->hwndFrame, + logf("CloseCurrentTab: tab: 0x%p win: 0x%p, hwndFrame: 0x%p, quitIfLast: %d\n", tab, win, win->hwndFrame, (int)quitIfLast); if (tab) { CloseTab(tab, quitIfLast); @@ -3388,7 +3388,7 @@ void CloseWindow(MainWindow* win, bool quitIfLast, bool forceClose) { if (win->isBeingClosed && !forceClose) { return; } - logf("CloseWindow: win: 0x%p, hwndFrame: 0x%x, quitIfLast: %d, forceClose: %d\n", win, win->hwndFrame, + logf("CloseWindow: win: 0x%p, hwndFrame: 0x%p, quitIfLast: %d, forceClose: %d\n", win, (void*)win->hwndFrame, (int)quitIfLast, (int)forceClose); win->isBeingClosed = true; ReportIf(forceClose && !quitIfLast); diff --git a/src/SumatraStartup.cpp b/src/SumatraStartup.cpp index ad857033ead..c648a29610a 100644 --- a/src/SumatraStartup.cpp +++ b/src/SumatraStartup.cpp @@ -234,8 +234,7 @@ static void OpenUsingDDE(HWND targetHwnd, const char* path, Flags& i, bool isFir i.startScroll.x != -1 && i.startScroll.y != -1) && isFirstWin) { const char* viewModeStr = DisplayModeToString(i.startView); - auto viewMode = ToWStrTemp(viewModeStr); - cmd.AppendFmt("[SetView(\"%s\", \"%s\", %.2f, %d, %d)]", fullPath, viewMode, i.startZoom, i.startScroll.x, + cmd.AppendFmt("[SetView(\"%s\", \"%s\", %.2f, %d, %d)]", fullPath, viewModeStr, i.startZoom, i.startScroll.x, i.startScroll.y); } if (i.forwardSearchOrigin && i.forwardSearchLine) { diff --git a/src/utils/StrconvUtil.cpp b/src/utils/StrconvUtil.cpp index a584419a77e..f1ad8ffce26 100644 --- a/src/utils/StrconvUtil.cpp +++ b/src/utils/StrconvUtil.cpp @@ -149,15 +149,14 @@ TempStr UnknownToUtf8Temp(const char* s, size_t cb) { if (str::StartsWith(s, UTF16_BOM)) { s += 2; int cch = (int)((len - 2) / 2); - // codeql complains about char* => WCHAR* cast - void* d = (void*)s; - return ToUtf8Temp((const WCHAR*)d, cch); + const WCHAR* ws = str::CastToWCHAR(s); + return ToUtf8Temp(ws, cch); } if (str::StartsWith(s, UTF16BE_BOM)) { // convert from utf16 big endian to utf16 s += 2; - WCHAR* ws = (WCHAR*)s; + WCHAR* ws = str::CastToWCHAR(s); int n = str::Leni(ws); WCHAR* tmpW = str::DupTemp(ws, n + 1); char* tmp = (char*)tmpW; diff --git a/src/utils/TgaReader.cpp b/src/utils/TgaReader.cpp index a7e19a2cdb5..254aa6bf666 100644 --- a/src/utils/TgaReader.cpp +++ b/src/utils/TgaReader.cpp @@ -397,7 +397,7 @@ ByteSlice SerializeBitmap(HBITMAP hbmp) { WORD w = (WORD)bmpInfo.bmWidth; WORD h = (WORD)bmpInfo.bmHeight; int stride = ((w * 3 + 3) / 4) * 4; - char* bmpData = AllocArrayTemp(stride * h); + char* bmpData = AllocArrayTemp(static_cast(stride) * static_cast(h)); if (!bmpData) { return {}; } @@ -452,7 +452,7 @@ ByteSlice SerializeBitmap(HBITMAP hbmp) { tgaData.Append((char*)&footerLE, sizeof(footerLE)); // don't compress the image data if that increases the file size - if (tgaData.size() > sizeof(headerLE) + w * h * 3 + sizeof(footerLE)) { + if (tgaData.size() > sizeof(headerLE) + static_cast(w) * h * 3 + sizeof(footerLE)) { tgaData.RemoveAt(0, tgaData.size()); headerLE.imageType = Type_Truecolor; tgaData.Append((char*)&headerLE, sizeof(headerLE));