Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sock_dns: Security issues (including remote code execution) #10739

Closed
nmeum opened this issue Jan 9, 2019 · 8 comments · Fixed by #10740
Closed

sock_dns: Security issues (including remote code execution) #10739

nmeum opened this issue Jan 9, 2019 · 8 comments · Fixed by #10740
Assignees
Labels
Area: network Area: Networking Impact: major The PR changes a significant part of the code base. It should be reviewed carefully Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@nmeum
Copy link
Member

nmeum commented Jan 9, 2019

Description

gnrc_sock_dns doesn't perform sufficient sanity checks on the DNS response it receives from the configured DNS server.

Here is a non-exhaustive list of issues:

  1. The QDCOUNT contained in the DNS response is not verified. This causes an out-of-bounds buffer access in the _skip_hostname function if QDCOUNT is set to a value larger than the buf len supplied to _parse_dns_reply.
  2. The RDLENGTH bounds-check for the answer section is incorrect for two reasons: (a) buf + len is the first invalid address, so >= needs to be used as a comparison operator (b) The result of bufpos + addrlen might cause a pointer overflow (especially due to the fact that addrlen is attacker controlled). If pointer overflows wrap around (undefined behaviour) this would allow an attacker to circumvent the bounds-check and exposes a buffer overflow vulnerability since the attacker controlled addrlen is later used in memcpy(addr_out, bufpos, addrlen), potentially allowing a code execution.
  3. The size of the caller allocated buffer addr_out is not passed to the _parse_dns_reply function at all. This makes checking whether the attacker controlled address actually fits in the buffer impossible and allows an easy buffer overflow and potential code execution.

All of these are especially critical due to the fact that DNS responses can easily be spoofed, especially since all spoofing protection mechanisms of DNS were not implemented. So an attacker doesn't even need to control the configured DNS server in order to exploit this.

Steps to reproduce the issue

  1. Flash an unmodified version of tests/gnrc_sock_dns to your RIOT node.
  2. Adjust your radvd.conf on your border router and add a RDNSS definition.

Causing a crash

Constantly send a DNS response with an excessive qdcount on the computer associated with the IP-Address you configured in the radvd RDNSS definition. For example

while sleep 1; do
    echo AACEAwkmAAAAAAAAKioqKioqKioqKioqKioqKioqKio= | \
        base64 -d | \
        socat fd:0 udp6-sendto:'[address-of-riot-node]':49152,sourceport=53
done

Remote code execution

This is (obviously) highly platform specific. We did this with BOARD=pba-d-01-kw2x. We wrote some ARM assembler code which toggles the LED and stored the machine code for it in the RDATA field of the answer section in the DNS response, thereby overflowing the addr buffer in the main stack frame. Our payload exactly fits into the stack frame of the main function and overwrites the return address of that function, jumping to the addr buffer and executing our payload.

Exploit written by @pyropeter. See: https://github.com/beduino-project/exploit-riot-dns

Versions

RIOT-Version: 5e03f58
Build environment:

Operating System Environment
-----------------------------
       Operating System: "Arch Linux" 
                 Kernel: Linux 4.19.4-arch1-1-ARCH x86_64 unknown

Installed compiler toolchains
-----------------------------
             native gcc: gcc (GCC) 8.2.1 20181127
      arm-none-eabi-gcc: arm-none-eabi-gcc (Arch Repository) 8.2.0
                avr-gcc: avr-gcc (GCC) 8.2.0
       mips-mti-elf-gcc: missing
             msp430-gcc: missing
   riscv-none-embed-gcc: missing
                  clang: clang version 7.0.0 (tags/RELEASE_700/final)

Installed compiler libs
-----------------------
   arm-none-eabi-newlib: "3.0.0"
    mips-mti-elf-newlib: missing
riscv-none-embed-newlib: missing
               avr-libc: "2.0.0" ("20150208")

Installed development tools
---------------------------
                  cmake: cmake version 3.13.1
               cppcheck: missing
                doxygen: 1.8.14
                 flake8: missing
                    git: git version 2.19.2
                   make: GNU Make 4.2.1
                openocd: Open On-Chip Debugger 0.10.0+dev-00436-g0fdf48f1 (2018-06-18-18:19)
                 python: Python 3.7.1
                python2: Python 2.7.15
                python3: Python 3.7.1
             coccinelle: missing
@miri64
Copy link
Member

miri64 commented Jan 9, 2019

@nmeum since this is a severe security flaw, we would have preferred some undisclosed heads up via security@riot-os.org.

@miri64 miri64 added the Area: security Area: Security-related libraries and subsystems label Jan 9, 2019
@miri64
Copy link
Member

miri64 commented Jan 9, 2019

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking Impact: major The PR changes a significant part of the code base. It should be reviewed carefully and removed Area: security Area: Security-related libraries and subsystems labels Jan 9, 2019
@miri64 miri64 added this to the Release 2019.01 milestone Jan 9, 2019
@miri64 miri64 changed the title gnrc_sock_dns: Security issues (including remote code execution) sock_dns: Security issues (including remote code execution) Jan 9, 2019
@miri64
Copy link
Member

miri64 commented Jan 9, 2019

(there is no gnrc_sock_dns so I assume you are referring to sock_dns)

@kaspar030
Copy link
Contributor

I'm AFK, can take a look later tonight.

@miri64
Copy link
Member

miri64 commented Jan 9, 2019

@kaspar030 already on it.

I can't reproduce the crash on native but from my debugging I would think it is just the enormous address space available there ;-).

@miri64
Copy link
Member

miri64 commented Jan 9, 2019

(I see that the _skip_hostname() loop is running wild ;-))

@miri64
Copy link
Member

miri64 commented Jan 9, 2019

Confirmed on samr21-xpro and pba-d-01-kw2x using ethos instead of gnrc_netdev_default (and copying the config from the border router example; I did not have a Raspberry Pi as border router at hand).

miri64 added a commit to miri64/RIOT that referenced this issue Jan 9, 2019
@miri64
Copy link
Member

miri64 commented Jan 9, 2019

Fixed in #10740

miri64 added a commit to miri64/RIOT that referenced this issue Jan 9, 2019
miri64 added a commit to miri64/RIOT that referenced this issue Jan 9, 2019
miri64 added a commit to miri64/RIOT that referenced this issue Jan 10, 2019
miri64 added a commit to miri64/RIOT that referenced this issue Jan 10, 2019
miri64 added a commit to miri64/RIOT that referenced this issue Jan 10, 2019
miri64 added a commit to miri64/RIOT that referenced this issue Jan 10, 2019
miri64 added a commit to miri64/RIOT that referenced this issue Jan 11, 2019
Fixes RIOT-OS#10739

(cherry picked from commit 2840b38)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Impact: major The PR changes a significant part of the code base. It should be reviewed carefully Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants