Ryan Giobbi from UPMC reported a header injection flaw in the Spacewalk web UI's return URL parameter: Request GET /rhn/systems/Overview.do?empty_set=true&return_url=67172%0d%0ad42e002fa0f HTTP/1.1 Host: host.example.com Accept: */* Accept-Language: en User-Agent: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.0) Connection: close Referer: https://host.example.com/rhn/YourRhn.do Cookie: pxt-session-cookie=7053xcace9e6d1158735e6f047ab49e4e509c; JSESSIONID=FAEED8F0E45715879B0D0AFACB8ADFF7 Response HTTP/1.0 302 Moved Temporarily Date: Thu, 21 Feb 2013 17:43:48 GMT Location: https://host.example.com/rhn/systems/67172 d42e002fa0f Content-Type: text/html;charset=UTF-8 Content-Length: 0 Set-Cookie: pxt-session-cookie=7053xcace9e6d1158735e6f047ab49e4e509c; Path=/; Secure; HttpOnly Connection: close
Acknowledgements: 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
commit c87e821ae7d3b27b6bfb32b7955bda327b3312a9
Reproduced. Working on a fix.
Created attachment 859232 [details] Close header-hole
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)
commit 3f3ce1687b5ae561bf5d8ae6450172fadfa2c2bb
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