After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 781120 - multiple memory leak in dict.c
multiple memory leak in dict.c
Status: RESOLVED INVALID
Product: libxml2
Classification: Platform
Component: general
git master
Other Linux
: Normal critical
: ---
Assigned To: Daniel Veillard
libxml QA maintainers
Depends on:
Blocks:
 
 
Reported: 2017-04-10 09:06 UTC by Vahagn vah_13 Vardanyan
Modified: 2017-04-11 06:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
./dict_h poc (1 bytes, text/plain)
2017-04-10 09:06 UTC, Vahagn vah_13 Vardanyan
Details

Description Vahagn vah_13 Vardanyan 2017-04-10 09:06:32 UTC
Created attachment 349592 [details]
./dict_h poc

the bug present because in the xmlDictLookup and xmlDictExists functions not checked the "name" length.

test1.c
```
#include <iostream>
#include <libxml/uri.h>
#include <fstream>

using namespace std;

int main(int argc, char* argv[]) {
    char *buffer = NULL;
    int string_size, read_size;
    FILE *handler = fopen(argv[1], "r");
    fseek(handler, 0, SEEK_END);
    string_size = ftell(handler);
    rewind(handler);
    buffer = (char*) malloc(sizeof(char) * (string_size + 1) );
    read_size = fread(buffer, sizeof(char), string_size, handler);
    buffer[string_size] = '\0';
    if (string_size != read_size)
    {
        free(buffer);
        buffer = NULL;
    }
    fclose(handler);
    xmlChar *aa = new xmlChar;
    *aa = *buffer;

    cout<<"length"<<strlen((char *)aa)+0x020ff0<<endl;
    xmlDictLookup(xmlDictCreate(),aa,(strlen((char *)aa)+0x020ff1));//!!
    cout<<"xmlDictLookupff"<<endl;
}
```
valgrind test1.c
```
==5834==    at 0x402EF88: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5834==    by 0x4588B23: memcpy (string3.h:51)
==5834==    by 0x4588B23: xmlDictAddString.isra.0 (dict.c:285)
==5834==    by 0x458CD6D: xmlDictLookup (dict.c:926)
==5834==    by 0x8048E2F: main (writer.cpp:46)
```


test2.c
```
#include <iostream>
#include <libxml/uri.h>
#include <fstream>

using namespace std;

int main(int argc, char* argv[]) {
    char *buffer = NULL;
    int string_size, read_size;
    FILE *handler = fopen(argv[1], "r");
    fseek(handler, 0, SEEK_END);
    string_size = ftell(handler);
    rewind(handler);
    buffer = (char*) malloc(sizeof(char) * (string_size + 1) );
    read_size = fread(buffer, sizeof(char), string_size, handler);
    buffer[string_size] = '\0';
    if (string_size != read_size)
    {
        free(buffer);
        buffer = NULL;
    }
    fclose(handler);

    xmlChar *aa = new xmlChar;
    *aa = *buffer;
    xmlDictExists(xmlDictCreate(),aa,(strlen((char *)aa)+0x020ff1));//!!
    cout<<"xmlDictExistsff"<<endl;
}
```
valgrind
```
==5940==    at 0x458F098: xmlDictComputeFastKey (dict.c:463)
==5940==    by 0x458F098: xmlDictExists (dict.c:988)
==5940==    by 0x8048D93: main (writer.cpp:55)
```
Comment 1 Vahagn vah_13 Vardanyan 2017-04-10 09:19:27 UTC
I think, need change code like this 

```
xmlDictLookup(xmlDictPtr dict, const xmlChar *name, int len) {
    unsigned long key, okey, nbi = 0;
    xmlDictEntryPtr entry;
    xmlDictEntryPtr insert;
    const xmlChar *ret;
    unsigned int l;

    if ((dict == NULL) || (name == NULL))
        return(NULL);

--    if (len < 0)
++    if (len < 0 || len > strlen((const char *) name))
        l = strlen((const char *) name);
    else
        l = len;

    if (((dict->limit > 0) && (l >= dict->limit)) ||
        (l > INT_MAX / 2))
        return(NULL);

    /*
     * Check for duplicate and insertion location.
     */
    okey = xmlDictComputeKey(dict, name, l);//!!

```
Comment 2 Daniel Veillard 2017-04-11 06:38:39 UTC
No that's a misuse of the API, if len is given it is assumed to be correct
http://xmlsoft.org/html/libxml-dict.html#xmlDictLookup

if we were to recompute the lenght all the time we should would not ask
for len to be provided.

Daniel