dmxAppConnect.js SAST vulnerability

I’m going through a CASA assessment for my app, and this vulnerability for dmxAppConnect.js was found. Can anything be done to resolve it?

Vulnerability description

Direct parsing from localStorage data without any previous validation could lead to injection attacks in desktop-app-v4/www/dmxAppConnect/dmxAppConnect.js

App Connect doesn’t get any data from localStorage, the only code where data from localStorage is used within dmxAppConnect.js is in the flow actions getStorage.

We do no validation since we don’t determine which data is being inserted. I’m not aware of any injection attack with JSON.parse which is used to parse the data from the localStorage.When your App doesn’t use the flow action getStorage it will never be called.

Do they give more information about the vulnerability and some solution for it?

They are pointing to this as the issue (Line 9 Col 81053) and categorizing it as a “Lack of data validation - Trust boundary violation

This is the one blocker that I’m unable to resolve easily to pass the CASA Tier 2 Assessment. Hopefully, there is some type of permanent fix that can be implemented. Otherwise, I may need to modify AppConnect to proceed with launching my app.

GPT provides the following guidance.


Here’s what you can do to improve the security of this code:

  1. Validate Key and Value : Ensure that the key and value used with localStorage are validated against expected formats or whitelisted values.
  2. Catch JSON Parsing Errors : When parsing JSON from localStorage , use a try...catch block to handle parsing errors gracefully. This helps to prevent issues if the data is not valid JSON.
  3. Sanitize Values : If the values retrieved from localStorage are used in the DOM or in a way that could execute code (like eval or innerHTML), sanitize these values to remove any executable code.

Here is an updated version of the code with added error handling for JSON parsing and a hypothetical sanitize function for the values:

setStorage(e) {
    const t = this.parse(e.key);
    const s = this.parse(e.value);
    if (typeof t !== 'string') throw new Error("setStorage: key must be a string");
    localStorage.setItem(t, JSON.stringify(s));
    return s;
},
getStorage(e) {
    const t = this.parse(e.key);
    if (typeof t !== 'string') throw new Error("getStorage: key must be a string");
    try {
        const item = localStorage.getItem(t);
        const parsedItem = JSON.parse(item);
        // Sanitize the parsedItem here if it's used in the DOM or evaluates code
        return parsedItem;
    } catch (err) {
        console.error('Error parsing JSON from localStorage', err);
        // Handle the error, perhaps return a default value
    }
},
removeStorage(e) {
    const t = this.parse(e.key);
    if (typeof t !== 'string') throw new Error("removeStorage: key must be a string");
    localStorage.removeItem(t);
    return true;
}

For the sanitize function, you will need to implement it based on how the data is used in your application. There are libraries like DOMPurify that help with sanitizing HTML content, but for other uses, you might need custom validation/sanitization logic.

2 Likes

Thanks Keith, great examples @patrick will check them out.

For html a custom app connect sanitizer might be required indeed.

1 Like

The flow actions already have error handling, it is done in the flow component that is calling the action methods. Also the sanitize is done on most places where values are used in the DOM, only the dmx-html attribute allows to insert unsafe html, all other DOM bindings are safe.

Do you use values from localStorage in the DOM and how do you bind them? We could include the DOMPurify for the dmx-html attribute. I know there are developers who do insert html with JavaScript and App Connect Components using the dmx-html attribute, so we should probably have the option to enable unsafe html for those developers. I think having it safe by default is the best way.

In the project that I submitted for the CASA assessment, I’m only storing one value for dark mode, but this is more about the potential vulnerability than immediate use in my project. Since App Connect is a core part of everything I’ve built, it appears I will need to prove that it’s not a vulnerability.

To be frank, the entire process of going through the assessment is challenging. Google and the assessors do not provide any advice on ways to work around the findings or how to explain that it handles validation like you mentioned above, only that “you must pass all 73 requirements” and App Connect code shows that “direct parsing from localStorage data without any previous validation could lead to injection attacks”.

Hi @George and @patrick I wanted to circle back on this as I'm starting to focus on clearing potential vulnerabilities.

Is it possible to override or extend dmxAppConnect files to implement fixes for potential vulnerabilities that are found, so we're not relying on you to correct them?

As an example. Here's a potential vulnerability in dmxNotifications.js

And the suggested fix.

  1. Identify the parts of the code where user-controlled data is being assigned to innerHTML. In this case, it is in the _create method where i.innerHTML = e;.
  2. Use the sanitize-html library to sanitize the HTML content before assigning it to innerHTML. This will help prevent XSS vulnerabilities by removing potentially harmful scripts.
  3. Import the sanitize-html library at the top of your file with const sanitizeHtml = require('sanitize-html');.
  4. Modify the _create method to sanitize the HTML content. Replace i.innerHTML = e; with i.innerHTML = sanitizeHtml(e);.
  5. Review the sanitization options provided by sanitize-html to ensure that the necessary HTML elements and attributes are allowed for your use case. You can customize the sanitization by passing options to sanitizeHtml(e, options);.

By sanitizing the HTML content, you ensure that any user-controlled data is cleaned of potentially harmful scripts before being rendered in the DOM.

I understand that I could sanitize on insert/retrieval of the data, but Google requires all vulnerabilities to be resolved, so I'm looking for a way to accomplish this on the identified files.

The data is not coming directly from user-controlled data but from a data binding that you define as developer. Not sure if Google's vulnerability test correctly detects that. It is a different story if you bind data to it that comes from a user input, then you should sanitize/validate it first before inserting it in the DOM.

@patrick just a wild guess here but I recently added a script to add all events that get fired on a Wappler site to the console for debugging things and I think it also triggered an even whenever data was inserted into the dom. Is that the case? If so then it would be not too hard to build an extension that sanitizes data whenever that event gets fired.

Well theoretically you could hit this will seriously delay all DOM rendering and make the app/site very slow

I believe they are looking at any potential exploits. For example, someone's account or the service's servers are compromised, the attacker injects something like the code below in a database field that is then rendered on the electron devices.

<script> require('child_process').exec('rm -rf /'); </script>

I understand that I should sanitize data that goes into the database, but on the off chance that the database is compromised, we would want to ensure that all users are not exploited if such code existed in the database or via some other exploit.

Also, a lot of the data I'm working with comes from third-party apis, so it should be considered untrusted input.

Yes but if there is a unique event for just the dmx-html data then it would not. I did not test this, yet and don’t know if it fires a unique event.

Saying this because I’ll have a call with someone tmr who wants to integrate an apple calendar. I know that’s doable especially with the new option to extend app connect with custom extensions but all the lions like Apple,Google,Meta etc have automated checks that need to be passed if you want to bring an app to enterprise. I know how to work around this. But still please give us as many options as you can when it comes to extending Wappler because that will help making it an enterprise production ready framework. Just my 2 cents again.

1 Like

bump

:crossed_fingers: :crossed_fingers: :crossed_fingers:

Bumping this again. Yes, I use data from local storage (i.e. SQLite) and would need the ability to define what script/tags to allow since I'm showing emails in a dmx-html.

As the suggested fix already explains you need to sanitize the html, you can create a custom formatter for App Connect that does this.

Formatter using sanitize-html

import sanitizeHtml from 'sanitize-html';

dmx.Formatter('string', 'sanitizeHtml', (html, options) => {
  return sanitizeHtml(html, options);
});

You can then use it in expressions like dmx-html="email.body.sanitizeHtml()"

They don't offer a browser ready script anymore, so you have to use some bundler to build the code for the formatter.