Potential Security Issue with Local Storage

[sderouchie]

Hello,

We have had a report of a Security issue within our product and after investigation the issue is due to internal SmartGWT javascript frameworks code.
In file ISC_Databinding.js there is code
getPriorityQueue:function(){
var pqText=this.get(this.priorityQueueKey);
if(pqText){
eval("var pq = "+pqText)}
else {
var pq=[]
}

The pqText string is retrieved from Offline Storage.
The complaint is that if the offline storage is modified to be “alert(1)” the alert will be displayed in the browser.

To reproduce this issue the security testing company did a manual change of the offline storage.

I tried their procedure on the SmartClient showcase (https://www.smartclient.com/smartcli...offlineSupport)
- go into developer tools
- Find isc-pq array. Edit it to be “alert(1)”
- click on the “Offline support” node in the left tree-grid
- the code is executed


I understand that there doesn’t seem to be a way to have the offline storage modified programmatically in the framework but this is one of those cases where we get reports of issues that “might” occur. They are saying that all strings should be validated.

Is this something that you would consider fixing? We have to respond to Security report we were given.

[Isomorphic]

So far as we know, the only way you would be able to modify something in localStorage would be to have the ability to execute arbitrary code in someone's browser. That's how you tested the "vulnerability" - using developer tools.

So this "security vulnerability" seems to say that, if you had the ability to execute arbitrary code in someone's browser, then via the localStorage system, you could then acquire the ability to execute arbitrary code in someone's browser... which you already had...

Please let us know if there is some other way to get data into localStorage, which might make this a real vulnerability. Otherwise, we would suggest to ignore this.

Just for background: we very often run into "security companies" that simply run "security scanner" products and pass along everything the scanner identifies, which is typically entirely spurious. You can spend all day chasing these, and it's all wasted effort.

On this particular instance - note that we could avoid the eval() here but in other, similar codepaths it would be extra work and substantially slower. If this is a false vulnerability, as we suspect, we could modify the framework to avoid triggering the security scanner, but it would need to be a consulting project in which we would downgrade the framework, just for you, to pass this spurious test.

[sderouchie]

These was my thoughts on the subject as well. Thank you for the detailed response; it should be helpful for us in providing a response to the complaint.