[RFC] msxml3: Fix crashes due to access to invalid context pointer.

[Bernhard Übelacker]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52073

In function xslt_doc_default_loader a pointer of the stack based
variable "xmlParserInputPtr input" is given to bind_url.

Later in function import_loader_onDataAvailable this pointer appears
as parameter "void *ctxt" which correctly gets casted to
"xmlParserInputPtr *input", but in my opinion incorrectly given
to xmlNewIOInputStream as parameter "xmlParserCtxtPtr ctxt".

In the next call to xmlNewInputStream this xmlParserCtxtPtr is
used to increment the input_id member.

By accident this input_id member contains the pointer which causes
in xmlXPathNodeCollectAndTest the segfault.
---
 dlls/msxml3/node.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/msxml3/node.c b/dlls/msxml3/node.c
index 721ad54e379..272e438e773 100644
--- a/dlls/msxml3/node.c
+++ b/dlls/msxml3/node.c
@@ -1329,7 +1329,7 @@ static HRESULT import_loader_onDataAvailable(void *ctxt, char *ptr, DWORD len)

inputbuffer = xmlParserInputBufferCreateIO(import_loader_io_read, import_loader_io_close, buffer,
             XML_CHAR_ENCODING_NONE);
-    *input = xmlNewIOInputStream(ctxt, inputbuffer, XML_CHAR_ENCODING_NONE);
+    *input = xmlNewIOInputStream(NULL, inputbuffer, XML_CHAR_ENCODING_NONE);
     if (!*input)
         xmlFreeParserInputBuffer(inputbuffer);

[Nikolay Sivov]

On 12/18/21 11:42, Bernhard Übelacker wrote:
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52073
>
> In function xslt_doc_default_loader a pointer of the stack based
> variable "xmlParserInputPtr input" is given to bind_url.
>
> Later in function import_loader_onDataAvailable this pointer appears
> as parameter "void *ctxt" which correctly gets casted to
> "xmlParserInputPtr *input", but in my opinion incorrectly given
> to xmlNewIOInputStream as parameter "xmlParserCtxtPtr ctxt".
>
> In the next call to xmlNewInputStream this xmlParserCtxtPtr is
> used to increment the input_id member.
>
> By accident this input_id member contains the pointer which causes
> in xmlXPathNodeCollectAndTest the segfault.

...
>   inputbuffer = xmlParserInputBufferCreateIO(import_loader_io_read, import_loader_io_close, buffer,
>           XML_CHAR_ENCODING_NONE);
> - *input = xmlNewIOInputStream(ctxt, inputbuffer, XML_CHAR_ENCODING_NONE);
> - *input = xmlNewIOInputStream(NULL, inputbuffer, XML_CHAR_ENCODING_NONE);
>  if (!*input)
>      xmlFreeParserInputBuffer(inputbuffer);

Hi,

thanks, this looks correct. According to libxml2 code, ctxt won't be 
useful anyway in this situation, even if we had a correct one.

[Bernhard Übelacker]

Am 20.12.21 um 09:08 schrieb Nikolay Sivov:
> On 12/18/21 11:42, Bernhard Übelacker wrote:
> > Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52073
> >
> > In function xslt_doc_default_loader a pointer of the stack based
> > variable "xmlParserInputPtr input" is given to bind_url.
> >
> > Later in function import_loader_onDataAvailable this pointer appears
> > as parameter "void *ctxt" which correctly gets casted to
> > "xmlParserInputPtr *input", but in my opinion incorrectly given
> > to xmlNewIOInputStream as parameter "xmlParserCtxtPtr ctxt".
> >
> > In the next call to xmlNewInputStream this xmlParserCtxtPtr is
> > used to increment the input_id member.
> >
> > By accident this input_id member contains the pointer which causes
> > in xmlXPathNodeCollectAndTest the segfault.
>
> ...
> > inputbuffer = 
> > xmlParserInputBufferCreateIO(import_loader_io_read, 
> > import_loader_io_close, buffer,
> >               XML_CHAR_ENCODING_NONE);
> > -    *input = xmlNewIOInputStream(ctxt, inputbuffer, 
> > XML_CHAR_ENCODING_NONE);
> > +    *input = xmlNewIOInputStream(NULL, inputbuffer, 
> > XML_CHAR_ENCODING_NONE);
> >       if (!*input)
> >           xmlFreeParserInputBuffer(inputbuffer);
>
> Hi,
>
> thanks, this looks correct. According to libxml2 code, ctxt won't be 
> useful anyway in this situation, even if we had a correct one.

Hello,
thanks for the review. Then I am going to submit it.

Kind regards,
Bernhard