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

Question Regarding Style Context #1512

Open
soaj1664 opened this issue Nov 1, 2014 · 12 comments
Open

Question Regarding Style Context #1512

soaj1664 opened this issue Nov 1, 2014 · 12 comments

Comments

@soaj1664
Copy link

soaj1664 commented Nov 1, 2014

Hey guys!

I had a comment. I am looking at the test-bed http://hoola.cz/nette-xss-test/?do=form-submit that you had created during earlier issue for Nette testing in different context.

My question is related to style context. If I input harmless style e.g., color:red then it should work but what I received as an output is

<div style='color\:red'>I am a style attribute context</div>

What's the point in escaping colon? I think the use case would be to allow simple styles without JavaScript execution. Isn't it? Or Am I missing something?

Thanks!

@milo
Copy link
Member

milo commented Nov 1, 2014

I'm not sure but... Latte does not tokenize CSS, CSS is one context. And colon escaping simple disallows you to pass javascript: into url().

@soaj1664
Copy link
Author

soaj1664 commented Nov 1, 2014

@milo But in the example case there is no JavaScript at all. It is a very simple and harmless style then why escape : in that case?

@milo
Copy link
Member

milo commented Nov 1, 2014

@soaj1664 As I wrote, Latte has only one CSS context. It supposes that variable contains CSS property value only, not the property name(s).

@soaj1664
Copy link
Author

soaj1664 commented Nov 1, 2014

@milo Isn't it a major weakness? It means Latte has an implicit assumption that every input is harmful.

<div style="Injection Lands Here">Style Context</div>

If it has an implicit assumption that every input is harmful then why at first place supporting style context at all?

@soaj1664
Copy link
Author

soaj1664 commented Nov 1, 2014

@milo I think what you said is that Latter support following

<div style="propertyname: INJECTION LANDS here or user controlled">Style Context</div>

@milo
Copy link
Member

milo commented Nov 1, 2014

Why weekness? Such implicit assumption seems secure to me.

You can use:
<div style="color:{$color}">...</div>

And the CSS context is not for tag attribute only. You can use Latte syntax in CSS files.

@soaj1664
Copy link
Author

soaj1664 commented Nov 1, 2014

It is not a weakness because now I understand what you wanted to say. I am wondering about : escaping and the confusion arise because of test-bed: see http://hoola.cz/nette-xss-test/?do=form-submit and look at style context. Can you fix the test-bed?

@milo
Copy link
Member

milo commented Nov 1, 2014

I'll migrate it to newer version soon, so I'll update it.

@soaj1664
Copy link
Author

soaj1664 commented Nov 1, 2014

Thanks! Please make sure that I can set the value of width:{input arrive here} and background-image:{input arrive here}. I want to test and will give you feedback.

@soaj1664
Copy link
Author

soaj1664 commented Nov 5, 2014

@milo Would you please tell me that have you find time to update the test-bed? One more thing I would like to have your take on is: e.g.,

<div style="color:{$color}">...</div>

if {$color} is #d0e4fe; (valid value for a color property) then Latte escapes # sign and will make harmless color's value non-render-able. Are you considered this a false positive? Here is the code I looked for escapeCSS function:

https://github.com/nette/latte/blob/master/src/Latte/Runtime/Filters.php#L84

Please correct me if I missed something. Thanks!

@Majkl578
Copy link
Contributor

Majkl578 commented Nov 5, 2014

This is an issue tracker, not a support forum.

@soaj1664
Copy link
Author

soaj1664 commented Nov 5, 2014

@Majkl578 So you do not considered a false positive an ISSUE :D

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

3 participants