security issues

[spaceone]

It looks https://github.com/SerkanSipahi/app-decorators/blob/master/src/libs/stylesheet.js#L481 is vulnerable to Cross Site Scripting (XSS) as the href link is not HTML attribute-value encoded.

It looks https://github.com/SerkanSipahi/app-decorators/blob/master/src/helpers/queryString.js#L96 is vulnerable to URL injection as the values of the query string aren't urlencoded.

[SerkanSipahi]

@spaceone you are right, many thanks! I will fix the XSS shortly. This is a freetime project. I will check it out after work.

[SerkanSipahi]

@spaceone There are two possible approaches to solve the XSS for: https://github.com/SerkanSipahi/app-decorators/blob/master/src/libs/stylesheet.js#L481

1.) The css compiler https://github.com/SerkanSipahi/app-decorators/blob/master/packages/postcss-parse-atrule-events can encode it for us so that we can save a lot of runtime computation or ...

2.) ... it will be encoded at runtime in the stylesheet.js lib itself. This can cause performance problems on complex, large application with many unique components that hold their css encapsulated.

=======================

This https://github.com/SerkanSipahi/app-decorators/blob/master/src/helpers/queryString.js#L96 can also be solved by the compiler

=======================

That actually is the idea behind app-decorators. It should solve a lot of things at compile time and not at runtime in the browser.

e.g. this: https://github.com/SerkanSipahi/app-decorators/blob/master/packages/babel-plugin-app-decorators-style-precompile/src/index.test.js#L76 compiles to: https://github.com/SerkanSipahi/app-decorators/blob/master/packages/babel-plugin-app-decorators-style-precompile/src/index.test.js#L104 and initialized then in: https://github.com/SerkanSipahi/app-decorators/blob/master/src/decorators/style.js#L69

Thank you for your "food for thought" in subject of XSS :1st_place_medal:

[spaceone]

I personally prefer variant 2 as the function might be called from different/multiple places which need to consider the escaping. It is the correct place where escaping should be done (it's the place where values is inserted into an HTML template). But I see your point, if this is only a function to build new source code. If you choose variant 1 you could make sure in the function itself that it's documented that the argument is expected to be already HTML encoded.

But for me it looks like if you put HTML encoding directly into the function that this happens at compile time. Doesn't the function returns new HTML code?

[SerkanSipahi]

If you develop with app-decorators you will never use the internal libs (queryString.js, stylesheet.js, etc) directly. There is no reason for it. Your point is correct! In most of the applications the place would be in variant 2 but this is a "Research Project". The goal of this project is to solve most of runtime computation at compile time. But if you really want have a one place encoding we can implement a solution where you could pass an option for runtime encoding instead of compiler encoding.

Because of app-decorators is an research project I would prefer the variant 1. They arent many places where you can use 'links' directly with app-dec. I dont mean something like dom.innerHTML = <a href="/foo/bar/bay.html">Hello World</a>. I mean, see below:

e.g. this

@style(\`
    @fetch http://testsite.test/<script>alert("xss");</script>
\`)
class Foo {}

compiles to:

@style([
     {
         attachOn:"immediately",
         imports:[
             "http://testsite.test/%3Cscript%3Ealert(%22TEST%22);%3C/script%3E"
         ],
         styles:'',
         type:"default"
     },
])
class Foo {}

or

@view(`
    <a href="http://testsite.test/<script>alert("xss");</script>"> ||| </a>
`)

@component()
class Sidebar {}

compiles to:

@view(`
    <a href="http://testsite.test/%3Cscript%3Ealert(%22xss%22);%3C/script%3E"> ||| </a>
`)

@component()
class Sidebar {}

As I said before this is a research project but im really happy about your "food for thought" in subject of XSS. Thank you.

See also: "The cost of Javascript" > https://medium.com/dev-channel/the-cost-of-javascript-84009f51e99e