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

Fixes #6580 - XSS in operating system name/description (CVE-2014-3531) #1580

Closed
wants to merge 1 commit into from

Conversation

dLobatog
Copy link
Member

The problem here is a misuse of .html_safe
.html_safe just turns the String into a SafeBuffer.

> os_name(operatingsystem).html_safe
"<img alt=\"Aix\" src=\"/assets/AIX.png\" />  T<script>alert(1)</script>O"
> os_name(operatingsystem).html_safe.class
ActiveSupport::SafeBuffer

That doesn't escape anything, but if we append anything to that string, the new string will be escaped.

> os_name(operatingsystem).html_safe + "<script></script>"
"<img alt=\"Aix\" src=\"/assets/AIX.png\" />  T<script>alert(1)</script>O&lt;script&gt;&lt;/script&gt;"

It's useful to avoid concatenations of tags, for instance one string ending with "<script>" and another one starting with "alert(1)</script>" will not concatenate into a real script.

In short, html_safe does not sanitize the input, it just tells Rails the string is "html_safe", but prevents future concatenations to make it unsafe.

"#{icon(record, opts)} #{record.to_label}".html_safe comes from user input and is not safe.

The current patch assumes 'family' in OS is safe. It does not come from user input but from a hash in the source app/models/operatingsystem.rb so I think it should be OK.

@lzap
Copy link
Member

lzap commented Jul 11, 2014

Taking for review

@lzap
Copy link
Member

lzap commented Jul 11, 2014

WFM +1

@lzap
Copy link
Member

lzap commented Jul 15, 2014

Merged as 98e584f for Foreman 1.6. Thanks @elobato

Do we backport to 1.5?

@lzap lzap closed this Jul 15, 2014
@dLobatog dLobatog deleted the 6580 branch July 29, 2014 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants