jackocnr / intl-tel-input

A JavaScript plugin for entering and validating international telephone numbers. React and Vue components also included.
https://intl-tel-input.com
MIT License
7.69k stars 1.95k forks source link

[vulnerability] Vulnerability Report - Potential Global Object Pollution vector #1845

Closed UmbraDeorum closed 1 month ago

UmbraDeorum commented 1 month ago

Plugin version

v24.6.0

Steps to reproduce

  1. Examine the code section provided below:
if (typeof module === 'object' && module.exports) {
    module.exports = factory();
} else {
    window.intlTelInput = factory();
}

Expected behaviour

Attaching to the global window object should be avoided, or at least a way to namespace it should be provided, so it’s not easily overwritten. A safer pattern would be to ensure that the object is only accessible within specific contexts and not globally. (e.g. avoid polluting the global namespace by encapsulating functionality within a closure.)

Actual behaviour

If the module is not being used in a CommonJS environment (such as Node.js), the object returned by the factory function is assigned to window.intlTelInput. This means that intlTelInput is accessible globally on the window object, and thus any JavaScript code running in the browser could potentially modify it. This issue consists a potential Global Object Pollution vector. A malicious actor could overwrite window.intlTelInput to inject malicious behavior, as the object is globally exposed. For example, if someone identifies that this global object is exposed, they could modify it as follows:

window.intlTelInput = function() {
    console.log("Malicious intlTelInput called!");
    return "maliciously modified data";
};

Or, in a more replayable manner:

localStorage.setItem('maliciousOverwrite', `
    window.intlTelInput = function() {
        console.log('Replayed malicious intlTelInput called!');
        return 'replayed malicious data';
    };
`);

if (localStorage.getItem('maliciousOverwrite')) {
    eval(localStorage.getItem('maliciousOverwrite'));
}

Initialisation options

None - issue identified through code review of the prebuilt js.

UmbraDeorum commented 1 month ago

The same issue seems to be present in earlier releases as well (e.g. v17.0.8), where assignment takes place using window.intlTelInput=a() .

jackocnr commented 1 month ago

This is standard practice for JS libraries - it's called UMD, and it allows a single build script to be consumed in different ways e.g. via import/require, or by just adding a script tag to your website (in this case there is no alternative than to use a global).