Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing HTTPOnly Flag for sysauth Cookie #32

Open
areynold opened this issue Sep 11, 2013 · 5 comments
Open

Missing HTTPOnly Flag for sysauth Cookie #32

areynold opened this issue Sep 11, 2013 · 5 comments
Assignees

Comments

@areynold
Copy link
Collaborator

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
Copy link
Collaborator Author

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.

@ghost ghost assigned areynold Sep 25, 2013
@areynold
Copy link
Collaborator Author

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

@areynold
Copy link
Collaborator Author

Fixed in luci-commotion 183c11ceba330407a4db7531fb86bf3a89126d7e.

Still needs upstream patch.

@areynold
Copy link
Collaborator Author

areynold commented Nov 1, 2013

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
Copy link
Collaborator Author

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.

@areynold areynold reopened this Dec 10, 2014
@jheretic jheretic removed this from the 10-2013 milestone Jan 8, 2015
@areynold areynold removed this from the 10-2013 milestone Jan 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants