smee icon indicating copy to clipboard operation
smee copied to clipboard

misbehaving DHCP client can crash boots

Open tobert opened this issue 3 years ago • 2 comments

Expected Behaviour

Boots shouldn't crash no matter what kind of traffic we throw at the DHCP server.

Current Behaviour

A dhcp client that sends a malformed packet can crash the service.

This example sends an invalid client GUID that causes a crash:

udhcpc -q -v PXEClient -x 0x5d:0000 -x 0x61:aaaaaaaa4a525bd43517df7f8b47

Possible Solution

Looks like it's in the dhcp4-go logging code. I did not dig into it.

Steps to Reproduce (for bugs)

Start boots standalone mode via docker-compose, then log into the client container with docker exec and run the offending udhcpc command.

# start boots in standalone mode
docker-compose up

# this should succeed
docker exec -ti boots_client_1 \
    busybox udhcpc -q -v PXEClient -x 0x5d:0000 -x 0x61:000000004a525bd43517df7f8b4799c18d

# this will crash boots
docker exec -ti boots_client_1 \
    busybox udhcpc -q -v PXEClient -x 0x5d:0000 -x 0x61:aaaaaaaa4a525bd43517df7f8b47

Context

I found this while coming up with udhcpc commands to test boots in docker-compose.

Your Environment

  • Operating System and version (e.g. Linux, Windows, MacOS):

Arch Linux

  • How are you running Tinkerbell? Using Vagrant & VirtualBox, Vagrant & Libvirt, on Packet using Terraform, or give details:

docker-compose

  • Link to your project or a code example to reproduce issue:

Everything you need is in main.

tobert avatar Aug 17 '21 16:08 tobert

Thanks Amy

mmlb avatar Aug 17 '21 21:08 mmlb

Eek! This bug is an accidental DoS attack in the making.

tstromberg avatar Aug 27 '21 03:08 tstromberg

Here's the panic from my environment:

panic: runtime error: slice bounds out of range [:16] with capacity 14

The formatUUID function of the logger is slicing the byte slice it receives as input without bounds checking: https://github.com/packethost/dhcp4-go/blob/master/logging.go#L91-L93

I'm happy to submit a fix. Would returning an unformatted version of the value received be preferable in this situation?

ryanfolsom avatar Oct 28 '22 01:10 ryanfolsom