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

Security update for lib/dust.js #194

Closed
wants to merge 15 commits into from
Closed

Security update for lib/dust.js #194

wants to merge 15 commits into from

Conversation

crypto
Copy link

@crypto crypto commented Nov 14, 2012

dust.backwardCompatibility = 2; // unit tests compatible
// localStorage.setItem('dust.backwardCompatibility', 0); // all important chars encoded
// localStorage.setItem('dust.backwardCompatibility', 1); // compatible with <script>var x = '{value|j}'</script>
// localStorage.setItem('dust.backwardCompatibility', 2); // unit tests compatible (usual encoding, vulnerable)

dust.backwardCompatibility special variable for if it's true \ not encoded in HTML and <&" not encoded in JS just backspaced "
We don't override JSON.stringify I just use dust.JSONstringify for js flag dust.JSONstringify({'test':'alert('1')'}) with backwardCompatibility is {"test":"<script\u003ealert\u0028'1'\u0029<\u002fscript\u003e"}
I added dust.sanitizeURI function with flag url (for example value='javascript:alert(1)'; {value|url|s} will be ./javascript:alert%281%29) you can use it with any valid url
I changed uc with dust.encodeURIComponent (with encoding of '()!*)
escapeHtml and escapeJs the same (except dust.backwardCompatibility, it additionally encode symbols like ()[]{}= etc. and replace some control chars)

console.log(dust.JSONstringify({x:text}));
console.log(dust.escapeJs(text));
console.log(dust.escapeHtml(text));
console.log(dust.encodeURIComponent(text));

{"x":"\u0000\u0001\u0002\u0003\u0004\u0005\u0006\u0007\b\t\n\u000b\f\r\u000e\u000f\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001a\u001b\u001c\u001d\u001e\u001f \u0021"\u0023$\u0025&\u0027\u0028\u0029\u002a+,-.\u002f0123456789:;<\u003d\u003e\u003f@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^\u0060abcdefghijklmnopqrstuvwxyz{|}~��"}
\u0000\u0001\u0002\u0003\u0004\u0005\u0006\u0007\u0008\t\n\u000b\u000c\r\u000e\u000f\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001a\u001b\u001c\u001d\u001e\u001f\u0020\u0021"\u0023\u0024\u0025&\u0027\u0028\u0029\u002a\u002b\u002c\u002d\u002e\u002f0123456789\u003a\u003b<\u003d\u003e\u003f\u0040ABCDEFGHIJKLMNOPQRSTUVWXYZ\u005b\u005c\u005d\u005e
\u0060abcdefghijklmnopqrstuvwxyz\u007b\u007c\u007d\u007e\u007f
���������
��
������������������ !"#$%&'()*+,-./0123456789:;<=>?@abcdefghijklmnopqrstuvwxyz[]^`abcdefghijklmnopqrstuvwxyz{|}~��
%00%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F%10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E
%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~%7F%C2%80

crypto added 15 commits October 30, 2012 16:53
Add few more characters to escapeJs, escapeHtml and JSON.stringify to prevent XSS attacks if developer use wrong encode context or use only one escape function if need 2 or more.
This function checks protocol and encode special symbols to URI + override encodeURIComponent to more secure version recommended by Mozilla
1. dust.backwardCompatibility special variable for if it's true \ not encoded in HTML and <'"& not encoded in JS just backspaced \' and \"
2. We don't override JSON.stringify I just use dust.JSONstringify for js flag
dust.JSONstringify({'test':'<script>alert(\'1\')</script>'})
with backwardCompatibility is {"test":"<script\u003ealert\u0028\'1\'\u0029<\u002fscript\u003e"}
3. I added dust.sanitizeURI function with flag url (for example value='javascript:alert(1)'; {value|url|s} will be ./javascript:alert%281%29) you can use it with any valid url
4. I changed uc with dust.encodeURIComponent (with encoding of '()!*)
5. escapeHtml and escapeJs the same (except dust.backwardCompatibility)
@vybs
Copy link
Contributor

vybs commented Jan 31, 2013

@rragan Do you have any comments on this PR?

We should try to get this into the Feb release. So I am going to follow up witha few members for a review.

@rragan
Copy link
Contributor

rragan commented Jan 31, 2013

I see backward compatibility flags being mentioned in the dust object. If we are going with 2.0.0, I strongly suggest taking a backward incompatibility hit to fix security issues. We don't want people turning off the fix and opening security holes.

Rich

From: Veena Basavaraj <[email protected]mailto:[email protected]>
Reply-To: linkedin/dustjs <[email protected]mailto:[email protected]>
Date: Thursday, January 31, 2013 12:37 PM
To: linkedin/dustjs <[email protected]mailto:[email protected]>
Cc: Microsoft Office User <[email protected]mailto:[email protected]>
Subject: Re: [dustjs] Security update for lib/dust.js (#194)

@rraganhttps://github.com/rragan Do you have any comments on this PR?

We should try to get this into the Feb release. So I am going to follow up witha few members for a review.


Reply to this email directly or view it on GitHubhttps://github.com//pull/194#issuecomment-12964360.

@vybs
Copy link
Contributor

vybs commented Feb 4, 2013

@rragan Sure, we are trying hard, just been busy with all the internal launches

We should be able to merge this to 2.0 in a week.

@rragan
Copy link
Contributor

rragan commented Feb 4, 2013

No problem. I know how internal stuff seems to take priority.

Rich

From: Veena Basavaraj [mailto:[email protected]]
Sent: Monday, February 04, 2013 12:21 PM
To: linkedin/dustjs
Cc: Ragan, Richard
Subject: Re: [dustjs] Security update for lib/dust.js (#194)

Sure, we are trying hard, just been busy with all the internal launches


Reply to this email directly or view it on GitHubhttps://github.com//pull/194#issuecomment-13097113.

@vybs
Copy link
Contributor

vybs commented Mar 20, 2013

@crypto is this ready for merge or review ?

if not can we close this PR as not actionable?

@vybs
Copy link
Contributor

vybs commented Apr 14, 2013

Closing this since we now have the dust-filters-secure repo

@vybs vybs closed this Apr 14, 2013
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

Successfully merging this pull request may close these issues.

3 participants