[CVE-2021-28458] Security Fix for Command Injection - huntr.dev

[huntr-helper]

@zpbrent (https://huntr.dev/users/zpbrent) has fixed a potential Command Injection vulnerability in your repository 🔨. For more information, visit our website (https://huntr.dev/) or click the bounty URL below...

Q | A Version Affected | * Bug Fix | YES Original Pull Request | https://github.com/418sec/ms-rest-nodeauth/pull/1

If you are happy with this disclosure, we would love to get a CVE assigned to the vulnerability. Feel free to credit @zpbrent, the discloser found in the bounty URL (below) and @huntr-helper.

User Comments:

📊 Metadata *

The core function execAz() which is purposely used for az command can be injected with arbitrary other OS commands. Also the attackers can exploit this vulnerability by calling AzureCliCredentials.setDefaultSubscription("OS command") from the Azure CLI.

Bounty URL: https://www.huntr.dev/bounties/1-npm-@azure/ms-rest-nodeauth/

⚙️ Description *

Using execFile() to replace exec().

💻 Technical Description *

The use of the child_process function exec() is highly discouraged if you accept user input and don't sanitize/escape them. This PR replaces it with execFile() which mitigates any possible Command Injections as it accepts input as arrays.

🐛 Proof of Concept (PoC) *

// PoC.js auth = require('@azure/ms-rest-nodeauth'); auth.AzureCliCredentials.setDefaultSubscription('$(touch pzhou@shu)');

🔥 Proof of Fix (PoF) *

Using 'execFile(az, [${cmd}, --out json], { encoding: "utf8" }, (error, stdout) => {...' to replacing 'exec(az ${cmd} --out json, { encoding: "utf8" }, (error, stdout) => {...'

👍 User Acceptance Testing (UAT)

@azure/ms-rest-nodeauth@3.0.7 test npm run build && run-p test:tslint test:unit

(node:7156) ExperimentalWarning: The fs.promises API is experimental

@azure/ms-rest-nodeauth@3.0.7 build run-s build:tsc build:rollup

(node:7179) ExperimentalWarning: The fs.promises API is experimental

@azure/ms-rest-nodeauth@3.0.7 build:tsc tsc -p tsconfig.json (node:7198) ExperimentalWarning: The fs.promises API is experimental @azure/ms-rest-nodeauth@3.0.7 build:rollup rollup -c rollup.config.js ./dist/lib/msRestNodeAuth.js → ./dist/msRestNodeAuth.js... created ./dist/msRestNodeAuth.js in 14ms (node:7232) ExperimentalWarning: The fs.promises API is experimental (node:7233) ExperimentalWarning: The fs.promises API is experimental @azure/ms-rest-nodeauth@3.0.7 test:tslint tslint -p . -c tslint.json @azure/ms-rest-nodeauth@3.0.7 test:unit mocha ✓ MSI App Service Authentication Credential getToken() should get token from the App service MSI by providing optional properties: 2ms ✓ MSI App Service Authentication Credential getToken() should get token from the App service MSI by reading the environment variables: 0ms ✓ MSI App Service Authentication Credential getToken() should throw if the response contains "ExceptionMessage": 0ms ✓ MSI App Service Authentication loginWithAppServiceMSI (callback) should successfully provide MSIAppServiceTokenCredentials object by providing optional properties: 0ms ✓ MSI App Service Authentication loginWithAppServiceMSI (callback) should successfully provide MSIAppServiceTokenCredentials object by reading the environment variables: 1ms ✓ MSI App Service Authentication loginWithAppServiceMSI (callback) should throw if the response contains "ExceptionMessage": 0ms ✓ MSI Vm Authentication should get token from the virtual machine with MSI service running at default endpoint: 1ms ✓ MSI Vm Authentication should get token from the virtual machine with MSI service running at custom endpoint: 1ms ✓ MSI Vm Authentication should throw on requests with bad resource: 0ms ✓ MSI Vm Authentication should throw on request with empty resource: 0ms ✓ MSI Vm Authentication should append schema to schema-less custom MSI endpoint: 0ms ✓ MSI Vm Authentication should set default api-version: 0ms ✓ MSI Vm Authentication should set custom api-version: 0ms ✓ MSI Vm Authentication should set default HTTP method: 0ms ✓ MSI Vm Authentication should set custom HTTP method: 0ms ✓ MSI Vm Authentication should set clientId as query parameter when provided: 0ms ✓ MSI Vm Authentication should set objectId as query parameter when provided: 0ms ✓ MSI Vm Authentication should set identityId as query parameter when provided: 0ms ✓ MSI Vm Authentication should set identityId, clientid and objectId as query parameter when provided: 0ms 19 passing (13ms)

🔗 Relates to...

https://www.huntr.dev/bounties/1-npm-@azure/ms-rest-nodeauth/

[ramya-rao-a]

@zpbrent To pass the CI, you will also need to

• Update the patch version in the package.json file
• Add an entry in the changelog file

cc @xirzec for review and follow ups on the CVE

[JamieSlome]

@ramya-rao-a & @xirzec - done, thanks! 🍰

[xirzec]

I'm working on getting a CVE number to stick in the changelog, so please hold for that before merging.

[xirzec]

CVE number issued: CVE-2021-28458. @JamieSlome @zpbrent thank you for your patience. Can one of you add this ID to the changelog so we can merge and publish?

[JamieSlome]

@xirzec - done, thanks for the CVE assignment! 🍰

[xirzec]

@ramya-rao-a can you give owner approval when you get a moment?

[ramya-rao-a]

Thanks @zpbrent @joheredi will be handling the release

[joheredi]

I'm working on the release now . I will post an update soon once this change has been released!

/cc: @ramya-rao-a, @zpbrent

[joheredi]

v3.0.8 has been released: https://www.npmjs.com/package/@azure/ms-rest-nodeauth/v/3.0.8

/cc: @ramya-rao-a, @zpbrent