Found a few XSS Vulnerabilities

[kaizentowfiq]

Hi everyone,

I hope you're all doing well.

I was doing a security review of version 1.11.9 of this external module before enabling it in our REDCap environments and stumbled upon a few XSS vulnerabilities. I believe these vulnerabilities are present regardless of what version of REDCap is being used.

It looks like the vulnerable lines of code are 84-86 in the config_translations.php file: https://github.com/vanderbilt-redcap/api-sync-module/blob/master/config_translations.php#L84 https://github.com/vanderbilt-redcap/api-sync-module/blob/master/config_translations.php#L85 https://github.com/vanderbilt-redcap/api-sync-module/blob/master/config_translations.php#L86

I believe $project_info["{$settings_prefix}project-name"], $project_info['url'], and $project_info['api-key'] need to be sanitized to prevent code execution before they're embedded in the returned HTML.

To reproduce the error, follow these steps:

1. Enable the external module on any project
2. Go to the project level settings of the enabled external module
3. Set the values for Remote REDCap URL, Project Name, and API Key to be <script>alert("injected alert from Remote REDCap URL EM configuration field")</script>, <script>alert("injected alert from Project Name configuration field")</script>, and <script>alert("injected alert from API Key configuration field")</script>. Note that any arbitrary javascript will execute here. The alerts are just to make it obvious that execution is happening in the browser.

5. Save the project level settings for the external module
6. Navigate to the API Sync - Configure Translations page

You should see something similar to these screenshots. The alert messages in the browser demonstrate that arbitrary javascript injected into the project level external module configuration fields persist in the system and execute whenever someone visits the API Sync - Configure Translations page.

XSS vulnerabilities like this have the potential to be pretty catastrophic, so we can't enable this module on our REDCap instances until these issues are fixed. I think applying htmlspecialchars() to each of the $project_info attributes on lines 84-86 is a sufficient fix. If I find any more vulnerabilities i'll let you know.

It might be a good idea to make a change at the External Module framework level to prevent these types of vulnerabilities from popping up. If REDCap sanitized External Module configuration parameters before storing them in the database, then these types of XSS vulnerabilities would not be present in External Modules regardless of any sanitization, or lack of sanitization, done in the External Module's code. To support any EMs that actually do want executable code in their configuration parameters, the External Module framework could support a code type to be used in the EM config.json project-settings or system-settings fields.

Thanks for reviewing this issue

[mmcev106]

@kaizentowfiq, thanks very much for the report! I plan to look into this in detail and release a new version of API Sync in the next week or so.

[kaizentowfiq]

Awesome, thank you @mmcev106

[mmcev106]

@kaizentowfiq, the changes you requested have been included in v1.11.10!

My primary job role is to attempt solve problems like this at the External Module Framework level. I'm not sure escaping data prior to insertion in the database is feasible. It not possible to ensure escaping for all possible database input sources (some are custom scripts at each institution). It is also likely not feasible to escape all data for all possible vulnerability types. Instead, we consider the database a "taint source". The psalm-based scan we perform on all new module submissions attempts to trace all possible data paths from the database to places in the code where vulnerability type is known (display, SQL queries, file operations, curl calls, etc.). This scan tool is also bundled with newer REDCap versions if you're curious (see Control Center -> External Modules -> Manage -> Module Security Scanning). It's not perfect, but it's constantly improving! I already have an item on the todo list for the next few months to consider module settings to be a taint source, which I believe would have caught these issues.

These type of concerns take a diligent community to stay on top of. Thanks very much for your help!

[remifrazierucsf]

Thank you, @mmcev106 !