[misc] AngularJs expression injection

[gabriellebourdages]

I know that ag-grid already covers HTML injection since #913 , but lately we found out that an XSS attack is still possible with a little bit more work when an application uses angular.

It is possible to escape expression sandboxing (See https://spring.io/blog/2016/01/28/angularjs-escaping-the-expression-sandbox-for-xss ) and inject code that can break the application.

Are there plans to address this issue eventually?

[ceolter]

no plans to address. if someone wants to suggest a PR we can consider, however this is the first time someone has pointed this out, which mean's its not a high priority for us (we prioritise things a lot of people ask for, and prioritise even more when it's paying customer).

[ceolter]

tracked by AG-158

[mbaluda]

This library is marked as vulnerable in the most used databases for tracking security issues.
https://nodesecurity.io/advisories/327
If it is like where I work, some of your prospect paying customers have specific policies in place for onboarding third party libraries that disqualify ag-grid alltogether because of this.

[smhc]

The exploit description at : https://spring.io/blog/2016/01/28/angularjs-escaping-the-expression-sandbox-for-xss
is a generic problem affecting all angular applications and libraries that misuse the expression sandbox. It is not a problem specific to ag-grid or a problem that ag-grid was exercising.

The expression sandbox was removed in angular 1.6 to help mitigate its misuse by developers:
http://angularjs.blogspot.com.au/2016/09/angular-16-expression-sandbox-removal.html

However... to quote that article:
"In all versions of Angular 1, your application is at risk of malicious attack if you generate Angular templates using untrusted user-provided content (even if the content is sanitized to contain no HTML).

ag-grid may be used to generate templates based on user input, but only if an application doesn't implement an angular-safe cell renderer. i.e it doesn't introduce any new vulnerability/risk that isn't already present when writing all angular applications. It is not a problem specific to ag-grid, but rather a problem with incorrectly written angular applications.

I suppose if a cell renderer was not configured and angular compilation is enabled, ag-grid could use a 'span ng-non-bindable' default cell renderer wrapper for safety/convenience. However this is easily mitigated by using a custom renderer that uses a safe template. If an ag-grid application is using angular compilation, in all likely-hood they will be using custom renderers.

Would it be possible to get written confirmation of this fact?

Advisories such as https://nodesecurity.io/advisories/327 seem to be naively generated from github issues without full investigation. This unfortunately impedes adoption by customers which have policies against third party software with outstanding advisories.

A documentation update to highlight this danger and ways to avoid the potential problem might be all that is required. If this issue is closed I assume the advisory would also be marked as resolved.

[storbeck]

we prioritise things a lot of people ask for

I'm also asking for this

[gabriellebourdages]

If anyone was still looking for a way to prevent this exploit, we ended up using a span ng-non-bindable tag (just like @smhc mentionned above) in a custom cell renderer that we applied to all our rows (not super convenient but it did the job).

[smhc]

@gabriellebourdages
You may be better off using < span ng-bind="::row.data[col]" >
or something to that effect. It prevents the possibility of user data closing the ng-non-bindable tag.

[CalvinTMurray]

This is a top priority for us as it is preventing our team from using ag-grid in our project because it is high risk. Any update on this?

[BrandonNoad]

I consider this high priority as well.

I also think the AngularJS section in the documentation should mention this possible vulnerability as well since a lot of users/applications may not be aware of it.

[smhc]

Perhaps @gabriellebourdages could close this issue given that it has been proven not to be an ag-grid problem, but rather a generic known angularjs pitfall. It would hopefully close that baseless advisory which is preventing adoption from enterprise customers.

It could be re-raised as a documentation issue if needed, without the words 'XSS vulnerability' to prevent bots scraping it unnecessarily.

[seanlandsman]

@smhc @gabriellebourdages thanks for the updates - we appreciate it

[kiril-matev]

Issue CVE-2017-16009 only applies for Angular JS i.e before Angular 2:
https://nvd.nist.gov/vuln/detail/CVE-2017-16009

AG Grid dropped support for AngularJs in v28 of the grid which is why this CVE is not applicable anymore.

This issue doesn't apply to AG Grid version 28 and later.

The CVE issue details have been updated to correctly reflect the versions as mentioned above.