This project is archived and is in readonly mode.

#3331 ✓stale
MOROHASHI Kyosuke

[PATCH] block invalid chars to come in rails app.

Reported by MOROHASHI Kyosuke | October 5th, 2009 @ 09:14 AM | in 2.3.10

Rails fixed the XSS vulnerability caused by invalid character.
This is the solution that new helper sanitize/cleanup invalid UTF-8
chars BEFORE OUTPUT to browser.

http://weblog.rubyonrails.org/2009/9/4/xss-vulnerability-in-ruby-on...

But it's better to block invalid chars BEFORE ACCEPT a (malformed) request.
We can identify that sending malformed UTF-8 as attack the app (as an
user, what's the request for?). We'd better hire "keep the barbarians
out at the gate" approach.

So I wrote a Rack middleware layer blocker.

This patch was tested with

  • ruby 1.8.7 / rails 2-3-stable / rack-1.0.0
  • ruby 1.8.7 / rails 2-3-stable / rack-trunk
  • ryby 1.9.2(trunk) / rails 2-3-stable / rack-trunk
  • Note: Rack 1.0.0 on Ruby 1.9 has a bug to fail parse multipart/form-data which contains binary body.

Comments and changes to this ticket

  • Jeremy Kemper

    Jeremy Kemper November 21st, 2009 @ 09:04 AM

    • Assigned user set to “Jeremy Kemper”
    • State changed from “new” to “open”
    • Milestone set to 2.3.6

    We chose before output because, for existing apps, barbarians are already through the gate. Blocking the gate is a good idea too. However, is malformed UTF-8 always an attack? Or should we just drop bad characters?

  • Matt Jones

    Matt Jones November 22nd, 2009 @ 03:59 AM

    I particularly wonder if IE 6 can be relied on to not send Windows-1252 and pretend that it's UTF-8. Doubly so with file uploads...

  • MOROHASHI Kyosuke

    MOROHASHI Kyosuke November 22nd, 2009 @ 04:32 PM

    Yes. I saw rails' "before output strategy", I think it's a good way, too. Both 'before accept' and 'before output' approach is not exclusive.

    I thought several times, but the case that user want to send an invalid UTF-8 were not found except for file uploads. And this patch takes care of file upload(if value is likely to IO, pass it). A user can't input invalid UTF-8 on their browsers.

    Of course, a user can send invalid cahrs using from script, but for why? (is'nt it attack?)

    IE6's behavior on Windows-1252 is new for me. in similer situation.

  • Matt Jones

    Matt Jones November 22nd, 2009 @ 07:35 PM

    • Tag cleared.

    Note that I'm not 100% sure that there is a problem with IE, but I've noticed that 80% of the encoding problems people run into (in CSV files, for instance) are the result of stray Windows-1252 characters. IE6 is the most likely suspect for problems simply because of its age relative to wide adoption of UTF-8.

    On further review of the patch, it appears that file uploads are passed through without issue, so that's one less thing to worry about.

  • Rizwan Reza

    Rizwan Reza May 16th, 2010 @ 02:41 AM

    • Tag set to bugmash
  • Jeremy Kemper

    Jeremy Kemper May 23rd, 2010 @ 05:54 PM

    • Milestone changed from 2.3.6 to 2.3.7
  • Jeremy Kemper

    Jeremy Kemper May 24th, 2010 @ 09:40 AM

    • Milestone changed from 2.3.7 to 2.3.8
  • Jeremy Kemper

    Jeremy Kemper May 25th, 2010 @ 11:45 PM

    • Milestone changed from 2.3.8 to 2.3.9
  • Jeremy Kemper

    Jeremy Kemper August 30th, 2010 @ 02:28 AM

    • Milestone changed from 2.3.9 to 2.3.10
    • Importance changed from “” to “”
  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:49 PM

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:49 PM

    • State changed from “open” to “stale”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Tags

Pages