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

Site save doesn't sanitize description for XML entities #49

Closed
gschneider-r7 opened this issue Feb 21, 2014 · 7 comments
Closed

Site save doesn't sanitize description for XML entities #49

gschneider-r7 opened this issue Feb 21, 2014 · 7 comments

Comments

@gschneider-r7
Copy link
Contributor

Line 263 in site.rb will sanitize site name, but not description:

xml = %(<Site id='#{id}' name='#{replace_entities(name)}' description='#{description}' riskfactor='#{risk_factor}'>)

This results in an error when trying to save a site that has any of the following in the description: & ' " < >

To reproduce:

Using the web UI in Nexpose:

  1. Create a site with any title
  2. Set the description to contain any of & ' " < >

Then, using a Ruby script with the gem:
3. Load the site
4. Save the site

@gschneider-r7
Copy link
Contributor Author

Using description='#{replace_entities(description)}' did resolve the issue in my tests.

@limhoff-r7
Copy link

Why is #to_xml building XML with Strings anyway?! Use Builder or Nokogiri::XML::Builder more robust that will handle the escaping for you.

@gschneider-r7
Copy link
Contributor Author

Looks like this can also happen with Credentials and Alerts.

@asalazar-r7
Copy link
Contributor

Yeah, looks like this code will benefit from being switched over to the XML objects instead of creating xml with strings.

@mdaines-r7
Copy link
Contributor

I'm pushing a quick fix for now. The string building is all over the place, so it will take some effort to clean up. As for Nokogiri, we don't want to introduce that dependency. But there is only one reason that I can think of for not switching to using the REXML libraries to do the same thing: If any code was depending upon the #to_xml calls to return a String, it will break if we switch over. That would require some more extensive testing internally just to make sure we don't break ourselves, let alone clients. A happy medium here might be to use the XML class, then to_s ourselves at the end?

@limhoff-r7
Copy link

If you want to support calling the XML objects in a nested manner you could use #as_xml to return REXML objects and #to_xml to return String. That would be similar to how #as_json being a Hash/Array and to_json being a String in Rails.

@mdaines-r7
Copy link
Contributor

Nice, I like that. That's something easy enough to add in over time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants