LinuxDC++

Invalid ADC commands sent via UDP will crash the app

Bug #1722364 reported by maksis on 2017-10-09

6

This bug affects 1 person

Affects		Status	Importance	Assigned to	Milestone
	AirDC++	Fix Released	Undecided	Unassigned
	ApexDC++	New	Undecided	Unassigned
	DC++	Fix Released	Undecided	Unassigned
	FlylinkDC++	New	Undecided	Unassigned
	LinuxDC++	New	Undecided	Unassigned
	StrongDC++	New	Undecided	Unassigned

Bug Description

The AdcCommand parsing function will throw ParseException on invalid commands: https://sourceforge.net/p/dcplusplus/code/ci/default/tree/dcpp/AdcCommand.cpp#l37

However, SearchManager (UDPServer in AirDC++) won't catch those exceptions at all: https://sourceforge.net/p/dcplusplus/code/ci/default/tree/dcpp/SearchManager.cpp#l286

As a result, you should be able to crash the app by sending the following raw text to the UDP port:

ARES T\n

(I used https://packetsender.com for testing)

Revision history for this message

maksis (maksis) wrote on 2017-10-10:

#1

AirDC++ now uses the regular dispatch function for parsing ADC UDP commands that should fix the issue: https://github.com/airdcpp/airgit/blob/6a40613788e7ed8a7478f4f1789bbd142a98d231/airdcpp/airdcpp/UDPServer.cpp#L130-L175

Changed in airdcpp:
status:	New → Fix Released

Revision history for this message

poy (poy) wrote on 2017-10-10:

#2

<https://github.com/airdcpp/airgit/commit/6a40613788e7ed8a7478f4f1789bbd142a98d231> makes perfect sense. a similar patch for DC++ would be very welcome. :)

nitpick: I'd change "public CommandHandler<UDPServer>" to private.

Revision history for this message

maksis (maksis) wrote on 2017-10-11:

#3

udp.patch Edit (10.6 KiB, text/plain)

Yeah, I took the code from AdcHub where it was public for some reason. Here's a patch for DC++.

Revision history for this message

poy (poy) wrote on 2017-10-11:

#4

what a nice patch! thanks, applied.
I especially liked the variadic template... ;)

- has anyting changed in the NMDC parsing part? hard to catch that from the patch.

- as I'm a bit rusty... could the "x[x.length() - 1] != 0x0a" check crash on empty data? it was after a "x.compare(1, 4, "RES ") == 0" check before your patch but that clause has been removed.

Changed in dcplusplus:
status:	New → Fix Committed

Revision history for this message

maksis (maksis) wrote on 2017-10-12:

#5

Ah right, the empty string check should have been added there as well (even though empty data seems to be currently discarded at https://sourceforge.net/p/dcplusplus/code/ci/f8540557bd5c28a9d0b3879b3e6b26f50ab670a6/tree/dcpp/SearchManager.cpp#l132). The old NMDC code was moved as it was as I don't even want to understand the logic in it.....

Revision history for this message

poy (poy) wrote on 2017-10-16:

#6

Fixed in DC++ 0.867.

Changed in dcplusplus:
status:	Fix Committed → Fix Released

eMTee (realprogger) on 2017-10-21

information type:

Private Security → Public

Report a bug

This report contains Public information

Everyone can see this information.

You are

Subscribing...

Edit bug mail

Other bug subscribers

Patches

udp.patch Edit

Add patch

Remote bug watches

Bug watches keep track of this bug in other bug trackers.