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 ef35ba8fdf14..a7135d3bcf05 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 c0e3b5bfa973..6ac3b9b3965c 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 91a7047f646c..2e7e824ec9c6 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 a7135d3bcf05..ca77a705681f 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 39c3dcaaf9fb..58b1d54275ed 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 d08794a2d35c..a3cd81dac143 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 1b40d76f954f..c2a5018eeb85 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 ad857033eadf..c648a29610ad 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 a584419a77ed..f1ad8ffce26a 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 a7e19a2cdb5d..254aa6bf6667 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));