The frame index should be strictly less than idCount. Previously, an index equal to idCount was not rejected, leading to an out-of-range access.
Signed-off-by: chenzhengyong chenzhengyong@uniontech.com
Although this bug was not fatal because subsequent IStream_Read and related functions also perform boundary checks, it is better to catch the invalid index early for clarity and consistency.
From: chenzhengyong chenzhengyong@uniontech.com
The frame index should be strictly less than idCount. Previously, an index equal to idCount was not rejected, leading to an out-of-range access.
Signed-off-by: chenzhengyong chenzhengyong@uniontech.com --- dlls/windowscodecs/icoformat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/windowscodecs/icoformat.c b/dlls/windowscodecs/icoformat.c index ae2ae115e5e..c6bc79d852e 100644 --- a/dlls/windowscodecs/icoformat.c +++ b/dlls/windowscodecs/icoformat.c @@ -662,7 +662,7 @@ static HRESULT WINAPI IcoDecoder_GetFrame(IWICBitmapDecoder *iface, goto fail; }
- if (This->header.idCount < index) + if (This->header.idCount <= index) { hr = E_INVALIDARG; goto fail;
I think we'd need an explicit test for this.
On Mon Nov 3 10:57:11 2025 +0000, Nikolay Sivov wrote:
I think we'd need an explicit test for this.
ok，I will add it later
On Mon Nov 3 10:57:11 2025 +0000, zhengyong chen wrote:
ok，I will add it later
Nikolay, I don't see why? I don't have a problem with more tests, but to me this seems like a case that should obviously be an error.
On Mon Nov 3 18:42:06 2025 +0000, Esme Povirk wrote:
Nikolay, I don't see why? I don't have a problem with more tests, but to me this seems like a case that should obviously be an error.
This won’t lead to a bug, as subsequent `IStream_Read` and related functions also include boundary checks. However, since boundary validation is necessary, it would be better to handle it correctly from the beginning.
-
chenzhengyong -
Esme Povirk (＠madewokherd)
-
Nikolay Sivov (＠nsivov)
-
zhengyong chen (＠chenzhengyong)