Amber: Missing input validation leads to XSS

[KINGSABRI]

Description

The example that was given in the documentation lacks user's input validation and make only client-side validation which is not security best practice more than UX

Steps to Reproduce

1. Follow amber auth example
2. Register a new user
3. Go to the user profile (http://0.0.0.0:3000/profile)
4. Run a web proxy (eg. burpsuite), intercept the connection
5. press "Update"
6. From the web proxy edit the email to be "><img src=x onerror=alert(1)>
7. Release the request
8. now refresh the page or even go to the home page, you'll see the popup
   

Expected behavior:
Since we're working on a framework people expected more security features such as CSRF protection, XSS protection, SQLi protection and so on.
In Rails, all inputs are sanitized by default. If developer insists to trust user inputs then he/she has to use html_safe() to force the trust (link).

Remediation

XSS is introduced into the application through untrusted user input. Most XSS threats can be avoided by: (1) Limiting both where and how untrustworthy input is used; and (2) ensuring that all user input is strongly validated and contextually
encoded.

Use positive whitelist validation for all user input. For example, use built-in optimized platform-specific or regular expressions). Be aware that regular expressions are CPU intensive, and that they can be used by attackers to tie up system resources in a denial-of-service attack.

If the application's framework supports casting, then ensure that all types (booleans, integers, floats, etc.) are cast. For rich input validation, an HTML policy engine, such as AntiSamy, must be used to ensure that the rich input does not contain spoofed content or XSS.

Encoding must be done contextually, making note of the
following contexts:

• String: validate and encode based on context
• CSS: validate and remove "expression" call, plus XSS
• HEX encode
• HTML Body: HTML Entity encode
• HTML Attribute: Aggressive HTML Entity Encoding,
• even encoding spaces
• URL Attribute: Validate that the URL is legal and 
• only contains safe protocols (and that the URL is specifically NOT a javascript:// url); then 
• attribute encode
• JavaScript: JS Output encode and 
• ensure certain functions like eval() or setTimeout() are not used.

XSS Prevention Rules
RULE #01 - Never Insert Untrusted Data Except in Allowed Locations
RULE #02 - HTML Escape Before Inserting Untrusted Data into HTML Element Content
RULE #03 - Attribute Escape Before Inserting Untrusted Data into HTML Common Attributes
RULE #04 - JavaScript Escape Before Inserting Untrusted Data into JavaScript Data Values
RULE #05 - HTML escape JSON values in an HTML context and read the data with JSON. parse
- JSON entity encoding
- HTML entity encoding
RULE #06 - CSS Escape And Strictly Validate Before Inserting Untrusted Data into HTML Style Property Values
RULE #07 - URL Escape Before Inserting Untrusted Data into HTML URL Parameter Values
RULE #08 - Sanitize HTML Markup with a Library Designed for the Job
RULE #09 - Prevent DOM-based XSS
RULE #10 - Use HTTPOnly cookie flag
RULE #11 - Implement Content Security Policy

References

• https://www.owasp.org/index.php/Reviewing_Code_for_Cross-site_scripting
• https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet
• https://www.owasp.org/index.php/DOM_based_XSS_Prevention_Cheat_Sheet
• http://projects.webappsec.org/w/page/13246920/Cross-Site-Scripting

Versions

• Crystal: Crystal 0.24.2 [4f9ed8d03] (2018-03-08)
• Amber: Amber CLI (amberframework.org) - v0.6.7
• OS: Ubuntu 17.10

Additional Information

Finally, I believe this is not the given example issue, it should be on the framework itself.

Thank you for making this framework available with such great documentation

[faustinoaq]

@KINGSABRI Thank you for reporting this issue! :+1:

/cc @amberframework/contributors

[faustinoaq]

Since we're working on a framework people expected more security features such as CSRF protection, XSS protection, SQLi protection and so on.

We already using CSRF protection, I think SQLinjection isn't an issue on amber.

XSS is a missing one, Maybe we can try using: https://github.com/rogeriozambon/http-protection

require "http/server"
require "http-protection"

server = HTTP::Server.new("0.0.0.0", 8080, [
  HTTP::Protection::Deflect.new,
  HTTP::Protection::FrameOptions.new,
  HTTP::Protection::IpSpoofing.new,
  HTTP::Protection::Origin.new,
  HTTP::Protection::PathTraversal.new,
  HTTP::Protection::RemoteReferrer.new,
  HTTP::Protection::StrictTransport.new,
  HTTP::Protection::XSSHeader.new
])

server.listen

WDYT?

[KINGSABRI]

Hello @faustinoaq,
Well, the suggested solution is not perfect all the time.
We always encourage people to not depend on the browser's protections as much as possible, because

1. it's sooner or later will be bypassed.
2. browser's support. Not all browsers support all security headers, link
3. security headers get obsolete, frequently.

It's fine to add these deaders, but it's not the proper solution for XSS issue. The proper solution is from within the framework itself to not trust user inputs by default. This makes framework users know they have the proper protection by default without a false sense of security.

Thank!

[faustinoaq]

Related issue here: https://github.com/amberframework/jasper-helpers/issues/3

[drujensen]

• SQL injection is avoided in both Granite and Crectco, if it's properly used
• CSRF protection is built into the forms if the pipe is included which it is by default
• CORS is provided by default for any API endpoint
• XSS protection via html escaping is provided if you use slang templating language. Unfortunately ecr does not provide protection.

[drujensen]

@KINGSABRI This took some time to figure out. Thanks for spending the time to report this and providing details on how to reproduce it. The burp suite tool is very easy to use.

In the navigation layout, the slang template was using a == instead of = for the email. This bypasses the sanitization of the fields i.e. html_safe

Let me know if you think there are any other security concerns.

[KINGSABRI]

@drujensen
Thank you for taking the time to fix the issue. The framework is promising and security should be part of its features.

Good luck guys, you're doing great!