CVE-2024-39935: Insufficient Information (NVD-CWE-noinfo)
value: jc21 NGINX Proxy Manager before 2.11.3 allows backend/internal/certificate.js OS command injection by an authenticated user (with certificate management privileges) via untrusted input to the DNS provider configuration. NOTE: this is not part of any NGINX software shipped by F5.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-07-04T21:15Z
Repository: https://github.com/NginxProxyManager/nginx-proxy-manager
Language: JavaScript
Files changed: backend/internal/certificate.js
Commit date: 2024-07-01 16:08:01 +1000
Commit message: Fix command injection when passing bash commands into the dns provider configuration

- Use built in node functions to write the file
- And to delete the file

Diff: diff --git a/backend/internal/certificate.js b/backend/internal/certificate.js
index 6033704..291056c 100644
--- a/backend/internal/certificate.js
+++ b/backend/internal/certificate.js
@@ -859,13 +859,12 @@ const internalCertificate = {
 		await certbot.installPlugin(certificate.meta.dns_provider);
 		const dnsPlugin = dnsPlugins[certificate.meta.dns_provider];
 		logger.info(`Requesting Let'sEncrypt certificates via ${dnsPlugin.name} for Cert #${certificate.id}: ${certificate.domain_names.join(', ')}`);
 
 		const credentialsLocation = '/etc/letsencrypt/credentials/credentials-' + certificate.id;
-		// Escape single quotes and backslashes
-		const escapedCredentials = certificate.meta.dns_provider_credentials.replaceAll('\'', '\\\'').replaceAll('\\', '\\\\');
-		const credentialsCmd     = 'mkdir -p /etc/letsencrypt/credentials 2> /dev/null; echo \'' + escapedCredentials + '\' > \'' + credentialsLocation + '\' && chmod 600 \'' + credentialsLocation + '\'';
+		fs.mkdirSync('/etc/letsencrypt/credentials', { recursive: true });
+		fs.writeFileSync(credentialsLocation, certificate.meta.dns_provider_credentials, {mode: 0o600});
 
 		// Whether the plugin has a --<name>-credentials argument
 		const hasConfigArg = certificate.meta.dns_provider !== 'route53';
 
 		let mainCmd = certbotCommand + ' certonly ' +
@@ -896,21 +895,19 @@ const internalCertificate = {
 
 		if (certificate.meta.dns_provider === 'duckdns') {
 			mainCmd = mainCmd + ' --dns-duckdns-no-txt-restore';
 		}
 
-		logger.info('Command:', `${credentialsCmd} && && ${mainCmd}`);
+		logger.info('Command:', mainCmd);
 
 		try {
-			await utils.exec(credentialsCmd);
 			const result = await utils.exec(mainCmd);
 			logger.info(result);
 			return result;
 		} catch (err) {
-			// Don't fail if file does not exist
-			const delete_credentialsCmd = `rm -f '${credentialsLocation}' || true`;
-			await utils.exec(delete_credentialsCmd);
+			// Don't fail if file does not exist, so no need for action in the callback
+			fs.unlink(credentialsLocation, () => {});
 			throw err;
 		}
 	},
 
 

Vulnerable code: 		// Escape single quotes and backslashes
		const escapedCredentials = certificate.meta.dns_provider_credentials.replaceAll('\'', '\\\'').replaceAll('\\', '\\\\');
		const credentialsCmd     = 'mkdir -p /etc/letsencrypt/credentials 2> /dev/null; echo \'' + escapedCredentials + '\' > \'' + credentialsLocation + '\' && chmod 600 \'' + credentialsLocation + '\'';
		logger.info('Command:', `${credentialsCmd} && && ${mainCmd}`);
			await utils.exec(credentialsCmd);
			// Don't fail if file does not exist
			const delete_credentialsCmd = `rm -f '${credentialsLocation}' || true`;
			await utils.exec(delete_credentialsCmd);
Fixed code: 		fs.mkdirSync('/etc/letsencrypt/credentials', { recursive: true });
		fs.writeFileSync(credentialsLocation, certificate.meta.dns_provider_credentials, {mode: 0o600});
		logger.info('Command:', mainCmd);
			// Don't fail if file does not exist, so no need for action in the callback
			fs.unlink(credentialsLocation, () => {});