[PATCH 0/3] MR4073: msxml3: Stop possible typelib out of bounds access (Coverity)

[Alistair Leslie-Hughes (＠alesliehughes)]

typelib has an array size of 2 (eg LibXml_Last), so a lookup
of IID_NULL will result in a lookup of the third index.

[PATCH 1/3] msxml3: Dont call qsort if we have no data (Coverity).

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 dlls/msxml3/dispex.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/msxml3/dispex.c b/dlls/msxml3/dispex.c
index cd7c2820133..6212fc2976c 100644
--- a/dlls/msxml3/dispex.c
+++ b/dlls/msxml3/dispex.c
@@ -294,9 +294,9 @@ static dispex_data_t *preprocess_dispex_data(DispatchEx *This)
         data->funcs = heap_realloc(data->funcs, data->func_cnt * sizeof(func_info_t));
     }

-    qsort(data->funcs, data->func_cnt, sizeof(func_info_t), dispid_cmp);
-
     if(data->funcs) {
+        qsort(data->funcs, data->func_cnt, sizeof(func_info_t), dispid_cmp);
+
         data->name_table = heap_alloc(data->func_cnt * sizeof(func_info_t*));
         for(i=0; i < data->func_cnt; i++)
             data->name_table[i] = data->funcs+i;

[PATCH 2/3] msxml3: Stop possible typelib out of bounds access (Coverity)

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

typelib has an array size of 2 (eg LibXml_Last), so a lookup
of IID_NULL will result in a lookup of the third index.
---
 dlls/msxml3/dispex.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/msxml3/dispex.c b/dlls/msxml3/dispex.c
index 6212fc2976c..ef68cb74745 100644
--- a/dlls/msxml3/dispex.c
+++ b/dlls/msxml3/dispex.c
@@ -82,7 +82,7 @@ static lib_id_t lib_ids[] = {
 };

static tid_id_t tid_ids[] = {
-    { &IID_NULL, LibXml_Last },
+    { &IID_NULL, LibXml2 },
     { &IID_IXMLDOMAttribute, LibXml2 },
     { &IID_IXMLDOMCDATASection, LibXml2 },
     { &IID_IXMLDOMComment, LibXml2 },

[PATCH 3/3] msxml3: Free IBindCtx on error paths (Coverity)

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 dlls/msxml3/httprequest.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/dlls/msxml3/httprequest.c b/dlls/msxml3/httprequest.c
index 459466a1234..953f49f1eee 100644
--- a/dlls/msxml3/httprequest.c
+++ b/dlls/msxml3/httprequest.c
@@ -736,6 +736,7 @@ static HRESULT BindStatusCallback_create(httprequest* This, BindStatusCallback *
             if (!(ptr = heap_alloc(size)))
             {
                 heap_free(bsc);
+                IBindCtx_Release(pbc);
                 return E_OUTOFMEMORY;
             }
             WideCharToMultiByte(cp, 0, str, len, ptr, size, NULL, NULL);
@@ -754,6 +755,7 @@ static HRESULT BindStatusCallback_create(httprequest* This, BindStatusCallback *
             {
                 SafeArrayUnaccessData(sa);
                 heap_free(bsc);
+                IBindCtx_Release(pbc);
                 return hr;
             }
             size++;
@@ -781,6 +783,7 @@ static HRESULT BindStatusCallback_create(httprequest* This, BindStatusCallback *
                     SafeArrayUnaccessData(sa);

heap_free(bsc);
+                IBindCtx_Release(pbc);
                 return E_OUTOFMEMORY;
             }

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/msxml3/dispex.c:
> };
>
> static tid_id_t tid_ids[] = {
> - { &IID_NULL, LibXml_Last },
> - { &IID_NULL, LibXml2 },
> { &IID_IXMLDOMAttribute, LibXml2 },
> { &IID_IXMLDOMCDATASection, LibXml2 },
> { &IID_IXMLDOMComment, LibXml2 },

How can this happen in practice?

[Alistair Leslie-Hughes (＠alesliehughes)]

On Wed Oct 11 10:03:13 2023 +0000, Nikolay Sivov wrote:
> How can this happen in practice?

I'm not sure if it's possible but it will remove the covering warning.

[Alistair Leslie-Hughes (＠alesliehughes)]

On Wed Oct 11 10:10:41 2023 +0000, Alistair Leslie-Hughes wrote:
> I'm not sure if it's possible but it will remove the covering warning.

It doesn't appear it can happen in practice.  There isn't any calls to use NULL_tid directly. Though NULL_tid is used as a end of loop for various tid_t structures (cdata.c/docfrag.c/etc)

We could just return an error in get_typeinfo for NULL_tid?

What would you prefer?

[Alex Henrie (＠alexhenrie)]

On Wed Oct 18 21:10:10 2023 +0000, Alistair Leslie-Hughes wrote:
> It doesn't appear it can happen in practice.  There isn't any calls to
> use NULL_tid directly. Though NULL_tid is used as a end of loop for
> various tid_t structures (cdata.c/docfrag.c/etc)
> We could just return an error in get_typeinfo for NULL_tid?
> What would you prefer?

You could change the first element of tid_ids from { &IID_NULL, LibXml_Last } to { NULL } to make it more clear that it is never used. Or, if we can't come to a consensus, you could mark the warning as a false positive in Coverity with a link to this discussion.