diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php index 41fac04f7..dc8f0d33f 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php @@ -57,7 +57,7 @@ class LeAccount extends LeCommon $this->setEnvironment(); // Store acme filenames - $this->acme_args[] = exec_safe('--home %s', self::ACME_HOME_DIR); + $this->acme_args[] = LeUtils::execSafe('--home %s', self::ACME_HOME_DIR); } /** @@ -118,8 +118,8 @@ class LeAccount extends LeCommon $acmecmd = '/usr/local/sbin/acme.sh ' . '--createAccountKey ' . implode(' ', $this->acme_args) . ' ' - . exec_safe('--accountkeylength %s', self::ACME_ACCOUNT_KEY_LENGTH) . ' ' - . exec_safe('--accountconf %s', $account_conf_file); + . LeUtils::execSafe('--accountkeylength %s', self::ACME_ACCOUNT_KEY_LENGTH) . ' ' + . LeUtils::execSafe('--accountconf %s', $account_conf_file); LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug); $proc = proc_open($acmecmd, $proc_desc, $proc_pipes, null, $proc_env); @@ -226,7 +226,7 @@ class LeAccount extends LeCommon $acmecmd = '/usr/local/sbin/acme.sh ' . '--registeraccount ' . implode(' ', $this->acme_args) . ' ' - . exec_safe('--accountconf %s', $this->account_conf_file); + . LeUtils::execSafe('--accountconf %s', $this->account_conf_file); LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug); $proc = proc_open($acmecmd, $proc_desc, $proc_pipes, null, $proc_env); diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php index b3251989f..ac238c2be 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php @@ -30,7 +30,6 @@ namespace OPNsense\AcmeClient; // Load legacy functions require_once("certs.inc"); // used in import() -require_once("util.inc"); // for exec_safe() use OPNsense\Core\Config; use OPNsense\AcmeClient\LeAccount; @@ -88,11 +87,11 @@ class LeCertificate extends LeCommon $this->cert_fullchain_file = (string)sprintf(self::ACME_FULLCHAIN_FILE, $this->config->id); // Store acme filenames - $this->acme_args[] = exec_safe('--home %s', self::ACME_HOME_DIR); - $this->acme_args[] = exec_safe('--certpath %s', $this->cert_file); - $this->acme_args[] = exec_safe('--keypath %s', $this->cert_key_file); - $this->acme_args[] = exec_safe('--capath %s', $this->cert_chain_file); - $this->acme_args[] = exec_safe('--fullchainpath %s', $this->cert_fullchain_file); + $this->acme_args[] = LeUtils::execSafe('--home %s', self::ACME_HOME_DIR); + $this->acme_args[] = LeUtils::execSafe('--certpath %s', $this->cert_file); + $this->acme_args[] = LeUtils::execSafe('--keypath %s', $this->cert_key_file); + $this->acme_args[] = LeUtils::execSafe('--capath %s', $this->cert_chain_file); + $this->acme_args[] = LeUtils::execSafe('--fullchainpath %s', $this->cert_fullchain_file); } /** @@ -437,7 +436,7 @@ class LeCertificate extends LeCommon $acmecmd = '/usr/local/sbin/acme.sh ' . '--remove ' . implode(' ', $this->acme_args) . ' ' - . exec_safe('--domain %s', (string)$this->config->name); + . LeUtils::execSafe('--domain %s', (string)$this->config->name); LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug); $proc = proc_open($acmecmd, $proc_desc, $proc_pipes, null, $proc_env); @@ -528,8 +527,8 @@ class LeCertificate extends LeCommon $acmecmd = '/usr/local/sbin/acme.sh ' . '--revoke ' . implode(' ', $this->acme_args) . ' ' - . exec_safe('--domain %s', (string)$this->config->name) . ' ' - . exec_safe('--accountconf %s', $account_conf_file); + . LeUtils::execSafe('--domain %s', (string)$this->config->name) . ' ' + . LeUtils::execSafe('--accountconf %s', $account_conf_file); LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug); $proc = proc_open($acmecmd, $proc_desc, $proc_pipes, null, $proc_env); diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeUtils.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeUtils.php index cdc642346..f0542f3d6 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeUtils.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeUtils.php @@ -4,7 +4,11 @@ * Copyright (C) 2017-2020 Frank Wall * Copyright (C) 2015 Deciso B.V. * Copyright (C) 2010 Jim Pingle + * Copyright (C) 2010 Ermal Luçi * Copyright (C) 2008 Shrew Soft Inc. + * Copyright (C) 2005-2006 Colin Smith + * Copyright (C) 2004-2007 Scott Ullrich + * Copyright (C) 2003-2004 Manuel Kasper * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -49,6 +53,27 @@ class LeUtils return rtrim(strtr(base64_encode($str), '+/', '-_'), '='); } + /** + * Properly escape arguments to prevent shell command injection. + * This is a copy of the exec_safe() legacy function. + * @param string $format The format string to use. + * @param string $args The arguments to escape. + * @return array|string The formatted and escaped arguments. + */ + public static function execSafe($format, $args = array()) + { + if (!is_array($args)) { + /* just in case there's only one argument */ + $args = array($args); + } + + foreach ($args as $id => $arg) { + $args[$id] = escapeshellarg($arg); + } + + return vsprintf($format, $args); + } + // Copied from system_camanager.php. public static function local_ca_import(&$ca, $str, $key = "", $serial = 0) { diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php index 8e0af7c36..5f2eb9169 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php @@ -34,9 +34,6 @@ use OPNsense\Core\Config; use OPNsense\AcmeClient\LeAccount; use OPNsense\AcmeClient\LeUtils; -// Load legacy functions -require_once("util.inc"); // for exec_safe() - /** * LeValidation stub file, contains shared logic for all validation methods. * @package OPNsense\AcmeClient @@ -82,8 +79,8 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon // Store acme hook switch ((string)$this->config->method) { case 'dns01': - $this->acme_args[] = exec_safe('--dns %s', (string)$this->config->dns_service); - $this->acme_args[] = exec_safe('--dnssleep %s', (string)$this->config->dns_sleep); + $this->acme_args[] = LeUtils::execSafe('--dns %s', (string)$this->config->dns_service); + $this->acme_args[] = LeUtils::execSafe('--dnssleep %s', (string)$this->config->dns_sleep); break; case 'http01': $this->acme_args[] = '--webroot /var/etc/acme-client/challenges'; @@ -91,11 +88,11 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon } // Store acme filenames - $this->acme_args[] = exec_safe('--home %s', self::ACME_HOME_DIR); - $this->acme_args[] = exec_safe('--certpath %s', sprintf(self::ACME_CERT_FILE, $this->cert_id)); - $this->acme_args[] = exec_safe('--keypath %s', sprintf(self::ACME_KEY_FILE, $this->cert_id)); - $this->acme_args[] = exec_safe('--capath %s', sprintf(self::ACME_CHAIN_FILE, $this->cert_id)); - $this->acme_args[] = exec_safe('--fullchainpath %s', sprintf(self::ACME_FULLCHAIN_FILE, $this->cert_id)); + $this->acme_args[] = LeUtils::execSafe('--home %s', self::ACME_HOME_DIR); + $this->acme_args[] = LeUtils::execSafe('--certpath %s', sprintf(self::ACME_CERT_FILE, $this->cert_id)); + $this->acme_args[] = LeUtils::execSafe('--keypath %s', sprintf(self::ACME_KEY_FILE, $this->cert_id)); + $this->acme_args[] = LeUtils::execSafe('--capath %s', sprintf(self::ACME_CHAIN_FILE, $this->cert_id)); + $this->acme_args[] = LeUtils::execSafe('--fullchainpath %s', sprintf(self::ACME_FULLCHAIN_FILE, $this->cert_id)); return true; } @@ -165,7 +162,7 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon $acmecmd = '/usr/local/sbin/acme.sh ' . "--${acme_action} " . implode(' ', $this->acme_args) . ' ' - . exec_safe('--accountconf %s', $account_conf_file); + . LeUtils::execSafe('--accountconf %s', $account_conf_file); LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug); $proc = proc_open($acmecmd, $proc_desc, $proc_pipes, null, $proc_env); @@ -215,7 +212,7 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon $key_length = $length; } - $this->acme_args[] = exec_safe('--keylength %s', $key_length); + $this->acme_args[] = LeUtils::execSafe('--keylength %s', $key_length); $this->cert_keylength = $length; } @@ -232,7 +229,7 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon $this->cert_challengealias = $challengealias; // Main domain for acme - $this->acme_args[] = exec_safe('--domain %s', $certname); + $this->acme_args[] = LeUtils::execSafe('--domain %s', $certname); // Main domain: Use DNS alias mode for domain validation? // https://github.com/Neilpang/acme.sh/wiki/DNS-alias-mode @@ -241,14 +238,14 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon case 'automatic': $name = '_acme-challenge.' . ltrim((string)$this->cert_name, '*.'); if ($dst = dns_get_record($name, DNS_CNAME)) { - $this->acme_args[] = exec_safe('--domain-alias %s', $dst[0]['target']); + $this->acme_args[] = LeUtils::execSafe('--domain-alias %s', $dst[0]['target']); } break; case 'domain': - $this->acme_args[] = exec_safe('--domain-alias %s', (string)$this->cert_domainalias); + $this->acme_args[] = LeUtils::execSafe('--domain-alias %s', (string)$this->cert_domainalias); break; case 'challenge': - $this->acme_args[] = exec_safe('--challenge-alias %s', (string)$this->cert_challengealias); + $this->acme_args[] = LeUtils::execSafe('--challenge-alias %s', (string)$this->cert_challengealias); break; } } @@ -256,7 +253,7 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon // altNames if (!empty((string)$this->cert_altnames)) { foreach (explode(",", (string)$this->cert_altnames) as $altname) { - $this->acme_args[] = exec_safe('--domain %s', $altname); + $this->acme_args[] = LeUtils::execSafe('--domain %s', $altname); // altNames: Use DNS alias mode for domain validation? // https://github.com/Neilpang/acme.sh/wiki/DNS-alias-mode @@ -265,14 +262,14 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon case 'automatic': $name = "_acme-challenge." . ltrim($altname, '*.'); if ($dst = dns_get_record($name, DNS_CNAME)) { - $this->acme_args[] = exec_safe('--domain-alias %s', $dst[0]['target']); + $this->acme_args[] = LeUtils::execSafe('--domain-alias %s', $dst[0]['target']); } break; case 'domain': - $this->acme_args[] = exec_safe('--domain-alias %s', (string)$this->cert_domainalias); + $this->acme_args[] = LeUtils::execSafe('--domain-alias %s', (string)$this->cert_domainalias); break; case 'challenge': - $this->acme_args[] = exec_safe('--challenge-alias %s', (string)$this->cert_challengealias); + $this->acme_args[] = LeUtils::execSafe('--challenge-alias %s', (string)$this->cert_challengealias); break; } } @@ -296,6 +293,6 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon */ public function setRenewal(int $interval = 60) { - $this->acme_args[] = exec_safe('--days %s', (string)$interval); + $this->acme_args[] = LeUtils::execSafe('--days %s', (string)$interval); } }