coverity / coverity-security-library

Coverity Security Library (CSL) is a lightweight set of escaping routines for fixing cross-site scripting (XSS), SQL injection, and other security defects in Java web applications.
http://security.coverity.com/document/2013/Mar/fixing-xss-a-practical-guide-for-developers.html
200 stars 38 forks source link

Proxy issue: U+003C (<) in a JS string context is escaped, increasing the output text size unnecessarily #2

Closed jpasski closed 11 years ago

jpasski commented 11 years ago

From Jim Manico via email. (Jim authorized the content of the email to be posted in this bug).

Email:

From: Jim Manico Subject: Escaper Date: January 19, 2013 2:36:37 PM PST To: Andy Chou, Jon Passki, Jeff Ichnowski, Jeremy Long

Andy and Jon,

CSL is remarkable, nice work gentlemen.

I think there are ways to make your JS encoder a bit more efficient (in terms of encoding output containing less characters).

Here is detailed explanation:

"(We think that) '<' is not the dangerous thing in JS, but '</' is.  Of course if you escape '<' then you escape '</', the solution in owasp > encoders is to escape '/' to '\/'.  Thus '' is encoded as '<\/script>' instead of '\x2c/script>' (or as the coverity case '\u002c/sc> ript>'> ).  The net result is saving 4 characters over coverity's unicode and 2 over the shorter hex-encoded alternative to coverity's.  In my experi> ence > though, '<' appears in a lot of JS strings, so it adds up." (from Jeff Ichnowski)>

This is a subtle point, but we wanted to let you know what we see what you are up to, it's awesome, and wanted to throw some feedback your way.

Last note, I really think the universe needs a Encoder test project.

I wanted to use some of your unit tests and Escaper research, and combine that with research from Jeff Williams (ESAPI), Jeff Ichnowski (OWASP > Encoder Project for Java), Mike Samuel (OWASP Java HTML Sanetizer and Google AppSec Lead for CAJA an others), Jeremy Long (Wells Fargo) as well> as > a few others who care about XSS Escaping, and form a working group.>

I am hoping that we can publish a language neutral test suite to check if anyone Escaper/Encoder class has been done in a secure fashion.

I write to you as a non profit board member of OWASP. That hat is 100% on. Please note I compete with you in the business world, and I'm > celebrating your work, providing a potential enhancement to it, and hoping we can all work together to serve the community.>

Aloha, Jim Manico OWASP Board Member @Manicode (808) 652-3805

neuroo commented 11 years ago

I agree that < itself is a nop for the JS string. We really care about few string literals that are related to HTML tags: </script>, <!--<script>, and </style>. Note that the latest should not be necessary, as this should be handled by the parent context escaper/sanitizer -- in this case we'd be in a RAWTEXT <style>. So in short, </ is good, but don't forget <! either. If you want to optimize even more, look for the previous string literals, and only escape in that case (obviously, in addition to quotes and stuff).

The proposal from Jim/OWASP seems fair (aka < appear in JS strings), but it really sounds like a micro-optimization. I understand that for some people the output size matters (aka Google, Facebook, etc.) but for most, they really don't care about how many bytes you generate from an escaper.

Also, we coud try to use the minimal escape sequence (mostly backslash and hex then, unicode only for U+2028/U+2029) for each escaped character, but it lacks consistency IMO, and that's not great for the devs. Easy enough though to have an option (or other version of the lib) that would generate the minimal escape sequences.

Jim, does OWASP have a ML or a place to discuss such subject? I couldn't find anything with the content of this discussion.

jpasski commented 11 years ago

Closing, timed out.