[PATCH 0/1] MR1006: gdi32: Fix possible overflow

[Mark Jansen (＠learn-more)]

According to the documentation of ScriptShape function, the psva argument should have the number of elements indicated by cMaxGlyphs.

[PATCH 1/1] gdi32: Fix possible overflow

[Mark Jansen]

From: Mark Jansen

According to the documentation of ScriptShape function, the psva argument should have the number of elements indicated by cMaxGlyphs.
---
 dlls/gdi32/text.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/gdi32/text.c b/dlls/gdi32/text.c
index f2fcb41bcdf..3cf829495c7 100644
--- a/dlls/gdi32/text.c
+++ b/dlls/gdi32/text.c
@@ -471,7 +471,7 @@ static BOOL BIDI_Reorder( HDC hDC,               /* [in] Display DC */
             WARN("Out of memory\n");
             goto cleanup;
         }
-        psva = HeapAlloc(GetProcessHeap(),0,sizeof(SCRIPT_VISATTR) * uCount);
+        psva = HeapAlloc(GetProcessHeap(),0,sizeof(SCRIPT_VISATTR) * cMaxGlyphs);
         if (!psva)
         {
             WARN("Out of memory\n");

[Bryan Kirk (＠Zero3K10)]

This fixes the crash that is happening in Character Mapper when scrolling through the list of fonts. It probably fixes other programs that show a preview of the font when scrolling through the list.

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/gdi32/text.c:
>          WARN("Out of memory\n");
>          goto cleanup;
>      }
> -
>    psva = HeapAlloc(GetProcessHeap(),0,sizeof(SCRIPT_VISATTR) * uCount);
> -
>    psva = HeapAlloc(GetProcessHeap(),0,sizeof(SCRIPT_VISATTR) * cMaxGlyphs);

`psva` would also need to be grown in the `while (res == E_OUTOFMEMORY)` loop after the first call to `ScriptShape()`.

Btw, patches that clean up the horrible mess that is `BIDI_Reorder()` would be most welcome ;-)

[Mark Jansen (＠learn-more)]

On Mon Oct 10 20:13:04 2022 +0000, Huw Davies wrote:
> `psva` would also need to be grown in the `while (res == E_OUTOFMEMORY)`
> loop after the first call to `ScriptShape()`.
> Btw, patches that clean up the horrible mess that is `BIDI_Reorder()`
> would be most welcome ;-)

Ah, I completely missed that.
Do you want me to amend this to the existing commit, or push a new commit?
Should I rebase on the latest master when pushing?

[Huw Davies (＠huw)]

On Tue Oct 11 13:56:49 2022 +0000, Mark Jansen wrote:
> Ah, I completely missed that.
> Do you want me to amend this to the existing commit, or push a new commit?
> Should I rebase on the latest master when pushing?

Please amend the exisiting commit, rebase, and force push.