Ryan Giobbi from UPMC reported a header injection flaw in the Spacewalk web UI's return URL parameter:
GET /rhn/systems/Overview.do?empty_set=true&return_url=67172%0d%0ad42e002fa0f HTTP/1.1
User-Agent: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.0)
Cookie: pxt-session-cookie=7053xcace9e6d1158735e6f047ab49e4e509c; JSESSIONID=FAEED8F0E45715879B0D0AFACB8ADFF7
HTTP/1.0 302 Moved Temporarily
Date: Thu, 21 Feb 2013 17:43:48 GMT
Set-Cookie: pxt-session-cookie=7053xcace9e6d1158735e6f047ab49e4e509c; Path=/; Secure; HttpOnly
Red Hat would like to thank Ryan Giobbi of UPMC for reporting this issue.
*** Bug 923465 has been marked as a duplicate of this bug. ***
Created attachment 820003 [details]
Patch to stop trusting return_url
Only allow return-url for 'local' URLs
Reproduced. Working on a fix.
Created attachment 859232 [details]
Patch "Close header-hole" checks for 0X0D (CR) but should also check for line feed (LF, 0X0A) just to be extra safe.
Few thoughts regarding the proposed additional fix:
- fix only checks for \r, it does not check for \n. Doing a quick test with just %0a instead of %0d%0a still gives me a response with header injected.
- it's probably good idea to avoid other special characters, such as all control characters. It may be possible to restrict values of return_url to a specific alphabet, or %-escape anything that is outside of some safe set of characters.
- consider ignoring return_url it if contains injected characters that can not be added there by Satellite. Unless there's real reason to try to use safe prefix, it's safer to ignore known-malicious input.
Created attachment 859668 [details]
Close header-hole, attempt #2
Kurt, Tomas - new patch, this one removes all control-characters. This seems to be the most efficient way to deal with attempted injection, without redesigning the redirect-after-relogin path entirely.
This should prevent header injection.
Is there a reason to try to sanitize problematic value rather than reject it? getLegalReturnUrl() already returns null if input does not have one of allowed prefixes. Why not do the same for bad return_url values with unexpected characters? Do you assume such may actually be generated by Satellite in some corner case?
Created attachment 859798 [details]
Close header hole, final
Kurt, Tomas - final patch to be submitted to QE today. Change from previous is to simply reject any retrun_url that contains control-characters (which means relogin drops you back to /rhn/YourRhn.do, the default starting location for Sat5)
This issue has been addressed in following products:
Red Hat Satellite Server v 5.6
Via RHSA-2014:0148 https://rhn.redhat.com/errata/RHSA-2014-0148.html