Fix important vulnerability in reg/ds.php + exescape

In page reg/ds.php, POST parameter 'key' was directly sent to shell, allowing for remote arbitrary commands execution.

This commit fixes this vulnerability, and uses a new function to automatically escape every shell command arguments as an additional generic protection.

fn/auth.php

@@ -106,13 +106,31 @@ function authDeleteUser($user_id) {
 		foreach (query('select', 'sites', ['username' => $user_id]) as $site)
 			htDeleteSite($site['address'], $site['type'], $user_id);
 
-		exec(CONF['ht']['sudo_path'] . ' -u ' . CONF['ht']['tor_user'] . ' ' . CONF['ht']['rm_path'] . ' -r ' . CONF['ht']['tor_keys_path'] . '/' . $user_id, result_code: $code);
+		exescape([
+			CONF['ht']['sudo_path'],
+			'-u',
+			CONF['ht']['tor_user'],
+		    '--',
+			CONF['ht']['rm_path'],
+			'-r',
+			'--',
+			CONF['ht']['tor_keys_path'] . '/' . $user_id,
+		], result_code: $code);
 		if ($code !== 0)
 			output(500, 'Can\'t remove Tor keys directory.');
 
 		removeDirectory(CONF['ht']['tor_config_path'] . '/' . $user_id);
 
-		exec(CONF['ht']['sudo_path'] . ' -u ' . CONF['ht']['sftpgo_user'] .  ' ' . CONF['ht']['rm_path'] . ' -r ' . CONF['ht']['ht_path'] . '/fs/' . $user_id, result_code: $code);
+		exescape([
+			CONF['ht']['sudo_path'],
+			'-u',
+			CONF['ht']['sftpgo_user'],
+			'--',
+			CONF['ht']['rm_path'],
+			'-r',
+			'--',
+			CONF['ht']['ht_path'] . '/fs/' . $user_id
+		], result_code: $code);
 		if ($code !== 0)
 			output(500, 'Can\'t remove user\'s directory.');
 	}

fn/common.php

@@ -16,6 +16,11 @@ function output($code, $msg = '', $logs = [''], $data = []) {
 	exit();
 }
 
+function exescape(array $args, array &$output = NULL, int &$result_code = NULL): int {
+	exec('2>&1 ' . implode(' ', array_map('escapeshellarg', $args)), $output, $result_code);
+	return $result_code;
+}
+
 function insert($table, $values) {
 	$query = 'INSERT INTO "' . $table . '"(';
 

fn/dns.php

@@ -15,42 +15,53 @@ function parseZoneFile($zone_content, $types, $filter_domain = false) {
 	return $parsed_zone_content;
 }
 
-function knotcConfExec($cmds) {
-	exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 conf-begin', $output['begin'], $code['begin']);
+function knotc(array $cmds, array &$output = NULL, int &$return_code = NULL): void {
+	exescape([
+		CONF['dns']['knotc_path'],
+		'--blocking',
+		'--timeout',
+		'3',
+		'--',
+		...$cmds,
+	], $output, $return_code);
+}
+
+function knotcConfExec(array $cmds): void {
+	knotc(['conf-begin'], $output['begin'], $code['begin']);
 	if ($code['begin'] !== 0)
 		output(500, 'knotcConfExec: <code>knotc</code> failed with exit code <samp>' . $code['begin'] . '</samp>: <samp>' . $output['begin'][0] . '</samp>.');
 
 	foreach ($cmds as $cmd) {
-		exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 conf-' . $cmd, $output['op'], $code['op']);
+		knotc($cmd, $output['op'], $code['op']);
 		if ($code['op'] !== 0) {
-			exec(CONF['dns']['knotc_path'] . ' --blocking conf-abort');
+			knotc(['conf-abort']);
 			output(500, 'knotcConfExec: <code>knotc</code> failed with exit code <samp>' . $code['op'] . '</samp>: <samp>' . $output['op'][0] . '</samp>.');
 		}
 	}
 
-	exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 conf-commit', $output['commit'], $code['commit']);
+	knotc(['conf-commit'], $output['commit'], $code['commit']);
 	if ($code['commit'] !== 0) {
-		exec(CONF['dns']['knotc_path'] . ' --blocking conf-abort');
+		knotc(['conf-abort']);
 		output(500, 'knotcConfExec: <code>knotc</code> failed with exit code <samp>' . $code['commit'] . '</samp>: <samp>' . $output['commit'][0] . '</samp>.');
 	}
 }
 
-function knotcZoneExec($zone, $cmd, $action = NULL) {
+function knotcZoneExec(string $zone, array $cmd, string $action = NULL) {
 	$action = checkAction($action ?? $_POST['action']);
 
-	exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 zone-begin ' . $zone, $output['begin'], $code['begin']);
+	knotc(['zone-begin', $zone], $output['begin'], $code['begin']);
 	if ($code['begin'] !== 0)
 		output(500, 'knotcZoneExec: <code>knotc</code> failed with exit code <samp>' . $code['begin'] . '</samp>: <samp>' . $output['begin'][0] . '</samp>.');
 
-	exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 zone-' . $action . 'set ' . $zone . ' ' . implode(' ', $cmd), $output['op'], $code['op']);
+	knotc(['zone-' . $action . 'set', $zone, ...$cmd], $output['op'], $code['op']);
 	if ($code['op'] !== 0) {
-		exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 zone-abort ' . $zone);
+		knotc(['zone-abort', $zone]);
 		output(500, 'knotcZoneExec: <code>knotc</code> failed with exit code <samp>' . $code['op'] . '</samp>: <samp>' . $output['op'][0] . '</samp>.');
 	}
 
-	exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 zone-commit ' . $zone, $output['commit'], $code['commit']);
+	knotc(['zone-commit', $zone], $output['commit'], $code['commit']);
 	if ($code['commit'] !== 0) {
-		exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 zone-abort ' . $zone);
+		knotc(['zone-abort', $zone]);
 		output(500, 'knotcZoneExec: <code>knotc</code> failed with exit code <samp>' . $code['commit'] . '</samp>: <samp>' . $output['commit'][0] . '</samp>.');
 	}
 }

fn/ht.php

@@ -9,7 +9,15 @@ function htSetupUserFs($id) {
 		output(500, 'Can\'t create user directory.');
 	if (chmod(CONF['ht']['ht_path'] . '/fs/' . $id, 0775) !== true)
 		output(500, 'Can\'t chmod user directory.');
-	exec(CONF['ht']['sudo_path'] . ' ' . CONF['ht']['chgrp_path'] . ' ' . CONF['ht']['sftpgo_group'] . ' ' . CONF['ht']['ht_path'] . '/fs/' . $id . ' --no-dereference', result_code: $code);
+	exescape([
+		CONF['ht']['sudo_path'],
+		'--',
+		CONF['ht']['chgrp_path'],
+		'--no-dereference',
+		'--',
+		CONF['ht']['sftpgo_group'],
+		CONF['ht']['ht_path'] . '/fs/' . $id,
+	], result_code: $code);
 	if ($code !== 0)
 		output(500, 'Can\'t change user directory group.');
 
@@ -20,7 +28,16 @@ function htSetupUserFs($id) {
 		output(500, 'Can\'t chmod Tor config directory.');
 
 	// Setup Tor keys directory
-	exec(CONF['ht']['sudo_path'] . ' -u ' . CONF['ht']['tor_user'] . ' ' . CONF['ht']['mkdir_path'] . ' --mode=0700 ' . CONF['ht']['tor_keys_path'] . '/' . $id, result_code: $code);
+	exescape([
+		CONF['ht']['sudo_path'],
+		'-u',
+		CONF['ht']['tor_user'],
+		'--',
+		CONF['ht']['mkdir_path'],
+		'--mode=0700',
+		'--',
+		CONF['ht']['tor_keys_path'] . '/' . $id,
+	], result_code: $code);
 	if ($code !== 0)
 		output(500, 'Can\'t create Tor keys directory.');
 }
@@ -93,19 +110,39 @@ function htDeleteSite($address, $type, $user_id) {
 			output(500, 'Failed to delete Tor configuration.');
 
 		// Reload Tor
-		exec(CONF['ht']['sudo_path'] . ' ' . CONF['ht']['tor_reload_cmd'], result_code: $code);
+		exescape([
+			CONF['ht']['sudo_path'],
+			'--',
+			...explode(' ', CONF['ht']['tor_reload_cmd']),
+		], result_code: $code);
 		if ($code !== 0)
 			output(500, 'Failed to reload Tor.');
 
 		// Delete Tor keys
-		exec(CONF['ht']['sudo_path'] . ' -u ' . CONF['ht']['tor_user'] . ' ' . CONF['ht']['rm_path'] . ' -r ' . CONF['ht']['tor_keys_path'] . '/' . $user_id . '/' . $dir, result_code: $code);
+		exescape([
+			CONF['ht']['sudo_path'],
+			'-u',
+			CONF['ht']['tor_user'],
+			'--',
+			CONF['ht']['rm_path'],
+			'-r',
+			'--',
+			CONF['ht']['tor_keys_path'] . '/' . $user_id . '/' . $dir,
+		], result_code: $code);
 		if ($code !== 0)
 			output(500, 'Failed to delete Tor keys.');
 	}
 
 	if ($type === 'dns') {
 		// Delete Let's Encrypt certificate
-		exec(CONF['ht']['sudo_path'] . ' ' . CONF['ht']['certbot_path'] . ' delete --quiet --cert-name ' . $address, result_code: $code);
+		exescape([
+			CONF['ht']['sudo_path'],
+			CONF['ht']['certbot_path'],
+			'delete',
+			'--quiet',
+			'--cert-name',
+			$address,
+		], result_code: $code);
 		if ($code !== 0)
 			output(500, 'Certbot failed to delete the Let\'s Encrypt certificate.');
 	}

fn/ns.php

@@ -50,14 +50,24 @@ function nsCheckZonePossession($zone) {
 
 function nsDeleteZone($zone, $user_id) {
 	// Remove from Knot configuration
-	knotcConfExec(["unset 'zone[$zone]'"]);
+	knotcConfExec([['conf-unset', 'zone[' . $zone . ']']]);
 
 	// Remove Knot zone file
 	if (unlink(CONF['ns']['knot_zones_path'] . '/' . $zone . 'zone') !== true)
 		output(500, 'Failed to remove Knot zone file.');
 
 	// Remove Knot related data
-	exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 --force zone-purge ' . $zone . ' +orphan', result_code: $code);
+	exescape([
+		CONF['dns']['knotc_path'],
+		'--blocking',
+		'--timeout',
+		'3',
+		'--force',
+		'--',
+		'zone-purge',
+		$zone,
+		'+orphan',
+	], result_code: $code);
 	if ($code !== 0)
 		output(500, 'Failed to purge zone data.');
 

jobs/check.php

@@ -12,7 +12,7 @@ const SUFFIX = 'test.servnest.test.';
 
 const TOR_PROXY = 'socks5h://127.0.0.1:9050';
 
-exec(CONF['dns']['kdig_path'] . ' torproject.org AAAA', $output, $return_code);
+exescape([CONF['dns']['kdig_path'], 'torproject.org', 'AAAA'], $output, $return_code);
 if (preg_match('/^;; Flags: qr rd ra ad;/Dm', implode("\n", $output)) !== 1)
 	exit('Unable to do a DNSSEC-validated DNS query.' . LF);
 
@@ -101,7 +101,12 @@ function testReg() {
 		'domain' => $domain,
 		'ns' => 'ns1.servnest.invalid.',
 	]);
-	exec(CONF['dns']['kdig_path'] . ' @' . CONF['reg']['address'] . ' ' . $domain . ' NS', $output);
+	exescape([
+		CONF['dns']['kdig_path'],
+		'@' . CONF['reg']['address'],
+		$domain,
+		'NS',
+	], $output);
 	if (preg_match('/[ \t]+ns1\.servnest\.invalid\.$/Dm', implode(LF, $output)) !== 1)
 		exit('Error: /reg/ns: NS record not set' . LF);
 
@@ -149,7 +154,12 @@ function testNs($domain) {
 		'tag' => 'issue',
 		'value' => 'letsencrypt.org',
 	]);
-	exec(CONF['dns']['kdig_path'] . ' @' . CONF['reg']['address'] . ' ' . $domain . ' CAA', $output);
+	exescape([
+		CONF['dns']['kdig_path'],
+		'@' . CONF['reg']['address'],
+		$domain,
+		'CAA',
+	], $output);
 	if (preg_match('/^' . preg_quote($domain, '/') . '[ \t]+7200[ \t]+IN[ \t]+CAA[ \t]+0[ \t]+issue[ \t]+"letsencrypt\.org"$/Dm', implode(LF, $output)) !== 1)
 		exit('Error: /ns/caa: CAA record not set' . LF);
 
@@ -158,7 +168,12 @@ function testNs($domain) {
 		'zone-content' => 'aaaa.' . $domain . ' 3600 AAAA ' . CONF['ht']['ipv6_address'] . "\r\n"
 			. '@ 86400 NS ' . CONF['ns']['servers'][0] . "\r\n",
 	]);
-	exec(CONF['dns']['kdig_path'] . ' @' . CONF['reg']['address'] . ' aaaa.' . $domain . ' AAAA', $output);
+	exescape([
+		CONF['dns']['kdig_path'],
+		'@' . CONF['reg']['address'],
+		'aaaa.' . $domain,
+		'AAAA',
+	], $output);
 	if (preg_match('/[ \t]+' . preg_quote(CONF['ht']['ipv6_address'], '/') . '$/Dm', implode(LF, $output)) !== 1)
 		exit('Error: /ns/edit: AAAA record not set' . LF);
 }

pg-act/ht/add-dns.php

@@ -30,7 +30,14 @@ checkAuthToken($matches[1], $matches[2]);
 
 rateLimit();
 
-exec('2>&1 ' . CONF['ht']['sudo_path'] . ' ' . CONF['ht']['certbot_path'] . ' certonly' . (($_SESSION['type'] === 'approved') ? '' : ' --test-cert') . ' --domain ' . $_POST['domain'], $output, $returnCode);
+exescape([
+	CONF['ht']['sudo_path'],
+	CONF['ht']['certbot_path'],
+	'certonly',
+	'--domain',
+	$_POST['domain'],
+	...(($_SESSION['type'] === 'approved') ? [] : ['--test-cert']),
+], $output, $returnCode);
 if ($returnCode !== 0)
 	output(500, 'Certbot failed to get a Let\'s Encrypt certificate.', $output);
 

pg-act/ht/add-onion.php

@@ -15,16 +15,29 @@ if (chmod($torConfFile, 0644) !== true)
 	output(500, 'Failed to give correct permissions to new Tor configuration file.');
 
 // Reload Tor
-exec(CONF['ht']['sudo_path'] . ' ' . CONF['ht']['tor_reload_cmd'], result_code: $code);
+exescape([
+	CONF['ht']['sudo_path'],
+	'--',
+	...explode(' ', CONF['ht']['tor_reload_cmd']),
+], result_code: $code);
 if ($code !== 0)
 	output(500, 'Failed to reload Tor.');
 
 usleep(10000);
 
 // Get the hostname generated by Tor
-$onion = exec(CONF['ht']['sudo_path'] . ' -u ' . CONF['ht']['tor_user'] . ' ' . CONF['ht']['cat_path'] . ' ' . CONF['ht']['tor_keys_path'] . '/' . $_SESSION['id'] . '/' . $_POST['dir'] . '/hostname', result_code: $code);
+exescape([
+	CONF['ht']['sudo_path'],
+	'-u',
+	CONF['ht']['tor_user'],
+	'--',
+	CONF['ht']['cat_path'],
+	'--',
+	CONF['ht']['tor_keys_path'] . '/' . $_SESSION['id'] . '/' . $_POST['dir'] . '/hostname',
+], $output, $code);
 if ($code !== 0)
 	output(500, 'Unable to read hostname file.');
+$onion = $output[0];
 if (preg_match('/^[0-9a-z]{56}\.onion$/D', $onion) !== 1)
 	output(500, 'No onion address found.');
 

pg-act/ns/edit.php

@@ -39,22 +39,22 @@ if (isset($_POST['zone-content'])) { // Update zone
 
 	ratelimit();
 
-	exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 zone-freeze ' . $_POST['zone'], $output, $return_code);
+	knotc(['zone-freeze', $_POST['zone']], $output, $return_code);
 	if ($return_code !== 0)
 		output(500, 'Failed to freeze zone file.', $output);
 
-	exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 zone-flush ' . $_POST['zone'], $output, $return_code);
+	knotc(['zone-flush', $_POST['zone']], $output, $return_code);
 	if ($return_code !== 0)
 		output(500, 'Failed to flush zone file.', $output);
 
 	if (file_put_contents(CONF['ns']['knot_zones_path'] . '/' . $_POST['zone'] . 'zone', $new_zone_content) === false)
 		output(500, 'Failed to write zone file.');
 
-	exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 zone-reload ' . $_POST['zone'], $output, $return_code);
+	knotc(['zone-reload', $_POST['zone']], $output, $return_code);
 	if ($return_code !== 0)
 		output(500, 'Failed to reload zone file.', $output);
 
-	exec(CONF['dns']['knotc_path'] . ' --blocking --timeout 3 zone-thaw ' . $_POST['zone'], $output, $return_code);
+	knotc(['zone-thaw', $_POST['zone']], $output, $return_code);
 	if ($return_code !== 0)
 		output(500, 'Failed to thaw zone file.', $output);
 

pg-act/ns/zone-add.php

@@ -5,7 +5,13 @@ $_POST['domain'] = formatAbsoluteDomain($_POST['domain']);
 if (query('select', 'zones', ['zone' => $_POST['domain']], 'zone') !== [])
 	output(403, _('This zone already exists on the service.'));
 
-exec(CONF['dns']['kdig_path'] . ' ' . ltrim(strstr($_POST['domain'], '.'), '.') . ' NS +short' . (CONF['ns']['local_only_check'] ? (' @' . CONF['reg']['address']) : ''), $parentAuthoritatives, $code);
+exescape([
+	CONF['dns']['kdig_path'],
+	ltrim(strstr($_POST['domain'], '.'), '.'),
+	'NS',
+	'+short',
+	...(CONF['ns']['local_only_check'] ? ['@' . CONF['reg']['address']] : []),
+], $parentAuthoritatives, $code);
 if ($code !== 0)
 	output(500, 'Unable to query parent name servers.');
 if ($parentAuthoritatives === [])
@@ -13,7 +19,13 @@ if ($parentAuthoritatives === [])
 foreach ($parentAuthoritatives as $parentAuthoritative)
 	checkAbsoluteDomainFormat($parentAuthoritative);
 
-exec(CONF['dns']['kdig_path'] . ' ' . $_POST['domain'] . ' NS @' . (CONF['ns']['local_only_check'] ? CONF['reg']['address'] : $parentAuthoritatives[0]) . ' +noidn', $results);
+exescape([
+	CONF['dns']['kdig_path'],
+	$_POST['domain'],
+	'NS',
+	'@' . (CONF['ns']['local_only_check'] ? CONF['reg']['address'] : $parentAuthoritatives[0]),
+	'+noidn'
+], $results);
 if (preg_match('/^' . preg_quote($_POST['domain'], '/') . '[\t ]+[0-9]{1,8}[\t ]+IN[\t ]+NS[\t ]+(?<salt>[0-9a-f]{8})-(?<hash>[0-9a-f]{32})\._domain-verification\.' . preg_quote(SERVER_NAME, '/') . '\.$/Dm', implode(LF, $results), $matches) !== 1)
 	output(403, _('NS authentication record not found.'));
 
@@ -47,8 +59,8 @@ if (chmod($knotZonePath, 0660) !== true)
 	output(500, 'Failed to chmod new zone file.');
 
 knotcConfExec([
-	"set 'zone[" . $_POST['domain'] . "]'",
-	"set 'zone[" . $_POST['domain'] . "].template' 'servnest'",
+	['conf-set', 'zone[' . $_POST['domain'] . ']'],
+	['conf-set', 'zone[' . $_POST['domain'] . '].template', 'servnest'],
 ]);
 
 output(200, _('Zone created.'));

pg-act/reg/ds.php

@@ -1,20 +1,18 @@
 <?php
 
-if (
-	($_POST['algo'] !== '8')
-	AND ($_POST['algo'] !== '13')
-	AND ($_POST['algo'] !== '14')
-	AND ($_POST['algo'] !== '15')
-	AND ($_POST['algo'] !== '16')
-) output(403, 'Wrong value for <code>algo</code>.');
+if (!in_array($_POST['algo'], ['8', '13', '14', '15', '16'], true))
+	output(403, 'Wrong value for <code>algo</code>.');
 
 $_POST['keytag'] = intval($_POST['keytag']);
-if ((!preg_match('/^[0-9]{1,6}$/D', $_POST['keytag'])) OR !($_POST['keytag'] >= 1) OR !($_POST['keytag'] <= 65535))
+if ((preg_match('/^[0-9]{1,6}$/D', $_POST['keytag'])) !== 1 OR !($_POST['keytag'] >= 1) OR !($_POST['keytag'] <= 65535))
 	output(403, 'Wrong value for <code>keytag</code>.');
 
 if ($_POST['dt'] !== '2' AND $_POST['dt'] !== '4')
 	output(403, 'Wrong value for <code>dt</code>.');
 
+if (preg_match('/^(?:[0-9a-fA-F]{64}|[0-9a-fA-F]{96})$/D', $_POST['key']) !== 1)
+	output(403, 'Wrong value for <code>key</code>.');
+
 regCheckDomainPossession($_POST['zone']);
 
 rateLimit();

pg-act/reg/transfer.php

@@ -11,7 +11,13 @@ $domain = formatAbsoluteDomain($_POST['subdomain'] . '.' . $_POST['suffix']);
 if (query('select', 'registry', ['username' => $_SESSION['id'], 'domain' => $domain], 'domain') !== [])
 	output(403, _('The current account already owns this domain.'));
 
-exec(CONF['dns']['kdig_path'] . ' ' . $domain . ' NS @' . CONF['reg']['address'] . ' +noidn', $results, $code);
+exescape([
+	CONF['dns']['kdig_path'],
+	$domain,
+	'NS',
+	'@' . CONF['reg']['address'],
+	'+noidn',
+], $results, $code);
 if ($code !== 0)
 	output(500, 'Unable to query registry\'s name servers.');
 if (preg_match('/^' . preg_quote($domain, '/') . '[\t ]+[0-9]{1,8}[\t ]+IN[\t ]+NS[\t ]+(?<salt>[0-9a-f]{8})-(?<hash>[0-9a-f]{32})\._transfer-verification\.' . preg_quote(SERVER_NAME, '/') . '\.$/Dm', implode(LF, $results), $matches) !== 1)

pg-view/reg/ds.php

@@ -51,7 +51,7 @@ foreach (regListUserDomains() as $domain)
 	<br>
 	<label for="key"><?= _('Key') ?></label>
 	<br>
-	<input id="key" required="" name="key" type="text" placeholder="018F25E4A022463478C9E30136EC53771A1704A0F0B3CE5B883AC9C8A6A55D16B638B4DE70662ACA5295D3669E7CADD9">
+	<input id="key" required="" name="key" type="text" pattern="^([0-9a-fA-F]{64}|[0-9a-fA-F]{96})$" placeholder="<?= strtoupper(bin2hex(random_bytes(32))) ?>">
 	<br>
 	<input type="submit" value="<?= _('Apply') ?>">
 </form>