pi-hole / FTL

The Pi-hole FTL engine
https://pi-hole.net
Other
1.36k stars 194 forks source link

Stack Buffer Overflow on unknown command #201

Closed segura2010 closed 6 years ago

segura2010 commented 6 years ago

In raising this issue, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your issue:

How familiar are you with the codebase?: 7


In https://github.com/pi-hole/FTL/blob/master/request.c#L151 the received client message is copied in "server_message" variable using "sprintf". "server_message" is a local variable with a maximum length given by "SOCKETBUFFERLEN" (1024 bytes -> https://github.com/pi-hole/FTL/blob/07b1275e532c273af0e8d9534ab54421892df6a1/FTL.h#L55).

If the client message is greater than 1024 bytes, it will overflow the buffer. The result is a stack buffer overflow. It is not exploitable because the binary is compiled with the latest security measures (ASLR, Stack Canary, PIE..). But it still is a way to cause a DDoS to the FTL server.

[BUG | ISSUE] Steps to reproduce:

Connect to FTL and send a command with length greater than 1024.

Device specifics

Hardware Type: All devices are affected. OS: All devices are affected.

This template was created based on the work of udemy-dl.

DL6ER commented 6 years ago

Thank you for your report. We are aware of this issue and have fixed this already in the current development version of FTL by using dynamically allocated buffers instead of static buffers.

As the current development version is incompatible with the most recent released version, I have backported this fix for v2.13.2 on branch master-fix/ssend.

We are currently discussing whether we want to release another point release or if we want to keep it like this until FTL v3.0 is ready for release. Abusing this bug for DDoS seems quite hard in my understanding as FTL, by default, binds only to the loopback interface of the machine so attacks would have to be launched from the device FTL is running on itself which renders attack from the outside even more unlikely. This attack vector seems to be quite unlikely so I don't actually feel too much pressure in getting this fix out. As you say the stack overflow is not exploitable to run code injection or similar bad things.

What is your opinion?

segura2010 commented 6 years ago

Hi @DL6ER! I did not realize it was fixed in the development version, sorry..! :S As you say, it only binds to the loopback interface so there is no way to exploit it from outside of the machine.

I just realized that there was a bug and I wanted to tell you, because in the future it could become exploitable. Take your time fixing it, there is no danger :)

And as I said, sorry if I disturbed you with a no dangerous bug that is fixed in the development version. :S

DL6ER commented 6 years ago

No need to apologize for anything! We changed quite a few things so I don't think that it was too obvious for you to see this. Please also report anything you find in the future because it will help us to make FTL and even better software for all of us! :-)

dschaper commented 6 years ago

Fixed in #219