Crash in httpreadwrite.c

[AmarOk1412]

Hi

On latest release (and some previous ones), I see a crash in libupnp when downloading some Igd descriptions.

I don't have a scenario yet (I'll dig) but I have a stacktrace:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==2769021==ERROR: AddressSanitizer: SEGV on unknown address 0x000000008000 (pc 0x7fe2bdc5ae40 bp 0x7fe21a932efc sp 0x7fe21a932e70 T230)
==2769021==The signal is caused by a READ memory access.
http_SendMessage src/genlib/net/http/httpreadwrite.c:515
http_RequestAndResponse src/genlib/net/http/httpreadwrite.c:788
http_Download src/genlib/net/http/httpreadwrite.c:897
UpnpDownloadUrlItem src/api/upnpapi.c:3500
UpnpDownloadXmlDoc src/api/upnpapi.c:3519

where fmt seems to be nullptr in some cases.

Got it a second time with GDB:

http_SendMessage (info=info@entry=0x7fff2f84ef10, TimeOut=TimeOut@entry=0x7fff2f84eefc, fmt=0x20000 <error: Cannot access memory at address 0x20000>, fmt@entry=0x7fffe9a026d6 "b") at src/genlib/net/http/httpreadwrite.c:515
515     while ((c = *fmt++)) {
(gdb) p fmt
$5 = 0x20000 <error: Cannot access memory at address 0x20000>
(gdb) p buf_length
$9 = 232
(gdb) up
#1  0x00007fffe905b1cd in http_RequestAndResponse (destination=destination@entry=0x7fff2f84f020, 
    request=0x6110020e7ec0 "GET /8499cbfd-4c88-442d-a430-15402f41cb69.xml HTTP/1.1\r\nHOST: 192.168.50.145:35187\r\nDATE: Thu, 12 Jan 2023 15:47:00 GMT\r\nCONNECTION: close\r\nUSER-AGENT: Linux/6.0.8-200.fc36.x86_64, UPnP/1.0, Portable "..., request_length=232, req_method=req_method@entry=HTTPMETHOD_GET, timeout_secs=<optimized out>, timeout_secs@entry=30, response=response@entry=0x7fff2f84f0f0) at src/genlib/net/http/httpreadwrite.c:788
788     ret_code = http_SendMessage(
(gdb) p request
0x6110020e7ec0 "GET /8499cbfd-4c88-442d-a430-15402f41cb69.xml HTTP/1.1\r\nHOST: 192.168.50.145:35187\r\nDATE: Thu, 12 Jan 2023 15:47:00 GMT\r\nCONNECTION: close\r\nUSER-AGENT: Linux/6.0.8-200.fc36.x86_64, UPnP/1.0, Portable "...

[ingo-h]

Hi @AmarOk1412,

I don't have a scenario yet (I'll dig)

You just use the library as normal and suddenly it crashes? How often?

[AmarOk1412]

I don't think it's new. It's just that at the office, there is a looooot of devices and some NAS/Android TV announced them as a UPnP router.

I don't see this crash often, except on the office network and I think I see this ~ 2 times a week. That's why I don't have yet a particular scenario is pretty rare and hard to get. But it seems to happen on some conditions.

[ingo-h]

where fmt seems to be nullptr in some cases.

As far as I can see from the debugger log fmt is set to 0x20000. That's not a nullptr.

[AmarOk1412]

indeed but still incorrect memory causing a segv

[AmarOk1412]

I reproduced the crash (EXCLUDE_WEB_SERVER is true)

It's when num_written != buf_length. e.g.

@@@@@@@@@@@@@ nw 244
@@@@@@@@@@@@@ num_written 244
@@@@@@@@@@@@@ buf_length 224

So I have more bit written than the length of the message.

[ingo-h]

Can you please explain what I have to do on my pupnp git clone to provoke the crash? Calling what function is essential?

[AmarOk1412]

void
PUPnP::downLoadIgdDescription(const std::string& locationUrl)
{
    IXML_Document* doc_container_ptr = nullptr;
    int upnp_err = UpnpDownloadXmlDoc(locationUrl.c_str(), &doc_container_ptr);

where locationUrl is a IGD discovered. The rest of the stack is in first comment and for now I don't have a specific scenario (seems to be various router/NAS announcing upnp services)

[ingo-h]

From the stacktrace the function UpnpDownloadXmlDoc() is called first. It has the declaration:

/* \brief Downloads an XML document specified in a URL. */
int UpnpDownloadXmlDoc(
    /*! [in] URL of the XML document. */
    const char* url,
    /*! [out] A pointer in which to store the XML document. */
    IXML_Document** xmlDoc);

Can you please give a real url that I can use as example?

[AmarOk1412]

Sorry for the long delay, I didn't got this crash since 2 weeks. But for the URL, it was from devices in a private network so sadly not usable (NAS or TV shield in general)

[FadiSheh]

Hello, I think I have a similar issue. I don't have a specific scenario, it happens sporadicaly. Here's my backtrace:

#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=140715836614208) at ./nptl/pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=140715836614208) at ./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=140715836614208, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3  0x00007ffff6842476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x00007ffff68287f3 in __GI_abort () at ./stdlib/abort.c:79
#5  0x00007ffff68896f6 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7ffff69db943 "*** %s ***: terminated\n") at ../sysdeps/posix/libc_fatal.c:155
#6  0x00007ffff693676a in __GI___fortify_fail (msg=msg@entry=0x7ffff69db8e9 "buffer overflow detected") at ./debug/fortify_fail.c:26
#7  0x00007ffff69350c6 in __GI___chk_fail () at ./debug/chk_fail.c:28
#8  0x00007ffff69366ab in __fdelt_chk (d=<optimized out>) at ./debug/fdelt_chk.c:25
#9  0x0000555557871514 in sock_read_write ()
#10 0x00005555578736b3 in http_SendMessage ()
#11 0x00005555578737c9 in http_RequestAndResponse ()
#12 0x00005555578748b6 in http_Download ()
#13 0x00005555578694f5 in UpnpDownloadUrlItem ()
#14 0x000055555786955a in UpnpDownloadXmlDoc ()

[mrjimenez]

Hi @FadiSheh ,

In order to be useful to us, we need either a repeatable full code example, or a debug enabled backtrace, i.e., a backtrace generated with a library compiled with --enable-debug for example:

./bootstrap && \
./configure --enable-debug  --enable-shared --prefix=/home/mroberto/usr/libupnp CFLAGS="-Wconversion -Wchar-subscripts -Wall -Wextra -Wpointer-arith -Wwrite-strings -Wformat-security -Wmissing-format-attribute" && \
make clean > /dev/null && \
make -j8 > /dev/null && \
make install > /dev/null

Ideally, we need both :smile:

Regards, Marcelo.

[AmarOk1412]

the backtrace is the same from my first message.

Today I got it again (I didn't see it the previous months).

Url was http://192.168.1.1:5431/dyndev/uuid:a78d5f43-98bb-4f75-a038-7309fec5c327 and downloaded file is:

a78d5f43-98bb-4f75-a038-7309fec5c327.txt

[AmarOk1412]

Ok I took some time to investigate this more as I saw that again.

pupnp mostly relies on select(). select() uses FD_SET, however FD_SET use a different limit than ulimits -n (authorized files). Here ulimits -n != 1024. So select() will not like this.

A solution would be to use poll() instead. But if I patch pupnp for our side, would you accept a pull request?

[mrjimenez]

Pull requests are always welcome and will be analysed to be accepted.

Before looking at any code, I must say I tend to like the poll() solution better than the present select().