As mentioned in an update to my post on the HNAP bug in the DIR-890L, the same bug was reported earlier this year in the DIR-645, and a patch was released. D-Link has now released a patch for the DIR-890L as well.
The patches for both the DIR-645 and DIR-890L are identical, so I’ll only examine the DIR-890L here.
Although I focused on command injection in my previous post, this patch addresses multiple security bugs, all of which stem from the use of strstr to validate the HNAP SOAPAction header:
- Use of unauthenticated user data in a call to system (command injection)
- Use of unauthenticated user data in a call to sprintf (stack overflow)
- Unauthenticated users can execute privileged HNAP actions (such as changing the admin password)
So, did they remove the sprintf stack overflow?
Did they remove the call to system?
Of course not!
Are they using strcmp instead of strstr to validate the SOAPAction header?
Pfft, why bother?
Their fix to all these fundamental problems is to use the access function to verify that the SOAPAction is a valid, expected action by ensuring that the file /etc/templates/hnap/<SOAPAction>.php exists:
OK, that does at least prevent users from supplying arbitrary data to sprintf and system.
However, they’ve added another sprintf to the code before the call to access; their patch to prevent an unauthenticated sprintf stack overflow includes a new unauthenticated sprintf stack overflow.
But here’s the kicker: this patch does nothing to prevent unauthenticated users from executing completely valid administrative HNAP actions, because all it does is ensure that the HNAP action is valid. That’s right, their patch doesn’t even address all the bugs listed in their own security advisory!
But I guess nobody really cares that any unauthenticated user can query information about hosts on the internal network, view/change system settings, or reset the router to its factory defaults:
$ wget --header="SOAPAction: http://purenetworks.com/HNAP1/GetDeviceSettings/SetFactoryDefault" http://192.168.0.1/HNAP1