search

opentechinstitute / commotion-router

The build system for the OpenWRT-based Commotion firmware.
https://commotionwireless.net
GNU General Public License v3.0
121 stars 43 forks source link

Missing HTTPOnly Flag for sysauth Cookie #32

Open areynold opened 10 years ago

areynold commented 10 years ago

FINDING ID: iSEC-COMMO13-4

TARGETS: The lack of an HTTPOnly flag on the sysauth administrative session cookie.

DESCRIPTION: Cookies set by the administrative application are not protected by the HTTPOnly flag. This flag indicates to modern browsers the session token cannot be accessed via JavaScript. When set by the web application, this provides excellent defense in depth against session hijacking via Cross-Site Scripting (XSS).

EXPLOIT SCENARIO: An attacker locates an XSS vulnerability within the application and uses it to perform trivial admin session hijacking. The attacker then performs operations on the administrators behalf such as changing the current password (which does not require the current password) in addition to a number of other sensitive actions.

SHORT TERM SOLUTION: Use a documented method within LuCI to set the HTTPOnly flag (if it exists) or apply the flag when setting the sysauth cookie via luci.http.header.

LONG TERM SOLUTION: Review the Commotion web applications and attack surface for standard web application best practices (check https://www.owasp.org for resources). Consider implementing a regression test to ensure this issue does not resurface in future releases once fixed.

areynold commented 10 years ago

Sysauth cookie seems to be built in dispatcher.lua, line 385. Untested, but seems to be a likely starting point.

Can also be set at the theme level by adding <httpCookies httpOnlyCookies="true" /> to page header. See commotion-openwrt-theme.

areynold commented 10 years ago

dispatcher.lua code is duplicated in luci-commotion-linux's apps_controller.lua. Pull request opened.

areynold commented 10 years ago

Fixed in luci-commotion 183c11ceba330407a4db7531fb86bf3a89126d7e.

Still needs upstream patch.

areynold commented 10 years ago

This issue was created before the correct repo was identified, and was assigned to commotion-openwrt as a placeholder.

This issue has been addressed in commotion-openwrt by way of opentechinstitute/luci-commotion/pull/28.

Because luci-commotion-linux is a fork of luci, the same issue exists in that code, and is fixed in opentechinstitute/luci-commotion-linux/pull/1.

Since this should still be submitted as an upstream fix, I am closing this issue and reopening it in luci-commotion as a request to submit a patch to luci.

areynold commented 9 years ago

There's a regression in the Commotion 1.1 release. It looks like the luci core libs are being compiled instead of installed as full source, so the patch to dispatcher.lua is being rejected.

Short term fix would be to build the luci core libs as source and check that the patch still applies cleanly.

Long term fix would be to submit an upstream patch.