Merge pull request #888 from RaspAP/bugfix/security

Bugfix: command injection vulnerabilities
RaspAP · May 10, 2021 · 061d01c · 061d01c
2 parents 5a7b774 + 4841d93
commit 061d01c
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 22 deletions.
diff --git a/ajax/networking/get_netcfg.php b/ajax/networking/get_netcfg.php
@@ -7,7 +7,7 @@
 
 if (isset($interface)) {
     // fetch dnsmasq.conf settings for interface
-    exec('cat '. RASPI_DNSMASQ_PREFIX.$interface.'.conf', $return);
+    exec('cat '. escapeshellarg(RASPI_DNSMASQ_PREFIX.$interface.'.conf'), $return);
     $conf = ParseConfig($return);
 
     $dhcpdata['DHCPEnabled'] = empty($conf) ? false : true;

diff --git a/app/img/wifi-qr-code.php b/app/img/wifi-qr-code.php
@@ -41,7 +41,7 @@ function qr_encode($str)
 $password = qr_encode($password);
 
 $data = "WIFI:S:$ssid;T:$type;P:$password;$hidden;";
-$command = "qrencode -t svg -m 0 -o - " . mb_escapeshellarg($data);
+$command = "qrencode -t svg -m 1 -o - " . mb_escapeshellarg($data);
 $svg = shell_exec($command);
 
 $config_mtime  = filemtime(RASPI_HOSTAPD_CONFIG);

diff --git a/app/js/custom.js b/app/js/custom.js
@@ -178,8 +178,8 @@ Populates the DHCP server form fields
 Option toggles are set dynamically depending on the loaded configuration
 */
 function loadInterfaceDHCPSelect() {
-    var iface = $('#cbxdhcpiface').val();
-    $.get('ajax/networking/get_netcfg.php?iface='+iface,function(data){
+    var strInterface = $('#cbxdhcpiface').val();
+    $.get('ajax/networking/get_netcfg.php?iface='+strInterface,function(data){
         jsonData = JSON.parse(data);
         $('#dhcp-iface')[0].checked = jsonData.DHCPEnabled;
         $('#txtipaddress').val(jsonData.StaticIP);

diff --git a/includes/functions.php b/includes/functions.php
@@ -524,11 +524,10 @@ function mb_escapeshellarg($arg)
 {
     $isWindows = strtolower(substr(PHP_OS, 0, 3)) === 'win';
     if ($isWindows) {
-        $escaped_arg = str_replace(array('"', '%'), '', $arg);
+        return '"' . str_replace(array('"', '%'), '', $arg) . '"';
     } else {
-        $escaped_arg = str_replace("'", "'\\''", $arg);
+        return "'" . str_replace("'", "'\\''", $arg) . "'";
     }
-    return "\"$escaped_arg\"";
 }
 
 function dnsServers()

diff --git a/includes/hostapd.php b/includes/hostapd.php
@@ -59,9 +59,8 @@ function DisplayHostAPDConfig()
             }
         }
     }
-
     exec('cat '. RASPI_HOSTAPD_CONFIG, $hostapdconfig);
-    exec('iwgetid '. $_POST['interface']. ' -r', $wifiNetworkID);
+    exec('iwgetid '. escapeshellarg($_POST['interface']). ' -r', $wifiNetworkID);
     if (!empty($wifiNetworkID[0])) {
         $managedModeEnabled = true;
     }
@@ -213,11 +212,14 @@ function SaveHostAPDConfig($wpa_array, $enc_types, $modes, $interfaces, $status)
         $good_input = false;
     }
 
-    if ($_POST['wpa'] !== 'none' 
-        && (strlen($_POST['wpa_passphrase']) < 8 || strlen($_POST['wpa_passphrase']) > 63)
-    ) {
+    # NB: A pass-phrase is a sequence of between 8 and 63 ASCII-encoded characters (IEEE Std. 802.11i-2004)
+    # Each character in the pass-phrase must have an encoding in the range of 32 to 126 (decimal). (IEEE Std. 802.11i-2004, Annex H.4.1)
+    if ($_POST['wpa'] !== 'none' && (strlen($_POST['wpa_passphrase']) < 8 || strlen($_POST['wpa_passphrase']) > 63)) {
         $status->addMessage('WPA passphrase must be between 8 and 63 characters', 'danger');
         $good_input = false;
+    } elseif (!ctype_print($_POST['wpa_passphrase'])) {
+        $status->addMessage('WPA passphrase must be comprised of printable ASCII characters', 'danger');
+        $good_input = false;
     }
 
     if (isset($_POST['hiddenSSID'])) {

diff --git a/installers/common.sh b/installers/common.sh
@@ -213,7 +213,7 @@ function _create_hostapd_scripts() {
     # Move service control shell scripts
     sudo cp "$webroot_dir/installers/"service*.sh "$raspap_dir/hostapd" || _install_status 1 "Unable to move service control scripts"
     # Make enablelog.sh and disablelog.sh not writable by www-data group.
-    sudo chown -c root:"$raspap_user" "$raspap_dir/hostapd/"*.sh || _install_status 1 "Unable change owner and/or group"
+    sudo chown -c root:root "$raspap_dir/hostapd/"*.sh || _install_status 1 "Unable change owner and/or group"
     sudo chmod 750 "$raspap_dir/hostapd/"*.sh || _install_status 1 "Unable to change file permissions"
     _install_status 0
 }
@@ -228,7 +228,7 @@ function _create_lighttpd_scripts() {
     sudo cp "$webroot_dir/installers/"configport.sh "$raspap_dir/lighttpd" || _install_status 1 "Unable to move service control scripts"
     # Make configport.sh writable by www-data group
     echo "Changing file ownership"
-    sudo chown -c root:"$raspap_user" "$raspap_dir/lighttpd/"*.sh || _install_status 1 "Unable change owner and/or group"
+    sudo chown -c root:root "$raspap_dir/lighttpd/"*.sh || _install_status 1 "Unable change owner and/or group"
     sudo chmod 750 "$raspap_dir/lighttpd/"*.sh || _install_status 1 "Unable to change file permissions"
     _install_status 0
 }
@@ -310,8 +310,9 @@ function _install_adblock() {
     echo "Moving and setting permissions for blocklist update script"
     sudo cp "$webroot_dir/installers/"update_blocklist.sh "$raspap_dir/adblock" || _install_status 1 "Unable to move blocklist update script"
 
-    # Make blocklists and update script writable by www-data group
-    sudo chown -c root:"$raspap_user" "$raspap_dir/adblock/"*.* || _install_status 1 "Unable to change owner/group"
+    # Make blocklists writable by www-data group, restrict update scripts to root
+    sudo chown -c root:"$raspap_user" "$raspap_dir/adblock/"*.txt || _install_status 1 "Unable to change owner/group"
+    sudo chown -c root:root "$raspap_dir/adblock/"*.sh || _install_status 1 "Unable to change owner/group"
     sudo chmod 750 "$raspap_dir/adblock/"*.sh || install_error "Unable to change file permissions"
 
     # Create 090_adblock.conf and write values to /etc/dnsmasq.d
@@ -400,11 +401,11 @@ function _create_openvpn_scripts() {
     _install_log "Creating OpenVPN control scripts"
     sudo mkdir $raspap_dir/openvpn || _install_status 1 "Unable to create directory '$raspap_dir/openvpn'"
 
-   # Move service auth control & logging shell scripts
+    # Move service auth control & logging shell scripts
     sudo cp "$webroot_dir/installers/"configauth.sh "$raspap_dir/openvpn" || _install_status 1 "Unable to move auth control script"
     sudo cp "$webroot_dir/installers/"openvpnlog.sh "$raspap_dir/openvpn" || _install_status 1 "Unable to move logging script"
-    # Make scripts executable by www-data group
-    sudo chown -c root:"$raspap_user" "$raspap_dir/openvpn/"*.sh || _install_status 1 "Unable change owner and/or group"
+    # Restrict script execution to root user
+    sudo chown -c root:root "$raspap_dir/openvpn/"*.sh || _install_status 1 "Unable change owner and/or group"
     sudo chmod 750 "$raspap_dir/openvpn/"*.sh || _install_status 1 "Unable to change file permissions"
     _install_status 0
 }
@@ -490,9 +491,10 @@ function _move_config_file() {
         _install_status 1 "'$raspap_dir' directory doesn't exist"
     fi
 
+    # Copy config file and make writable by www-data group
     _install_log "Moving configuration file to $raspap_dir"
     sudo cp "$webroot_dir"/raspap.php "$raspap_dir" || _install_status 1 "Unable to move files to '$raspap_dir'"
-    sudo chown -R $raspap_user:$raspap_user "$raspap_dir" || _install_status 1 "Unable to change file ownership for '$raspap_dir'"
+    sudo chown -c $raspap_user:"$raspap_user" "$raspap_dir"/raspap.php || _install_status 1 "Unable change owner and/or group"
 }
 
 # Set up default configuration
@@ -506,8 +508,8 @@ function _default_configuration() {
         sudo cp $webroot_dir/config/dhcpcd.conf /etc/dhcpcd.conf || _install_status 1 "Unable to move dhcpcd configuration file"
         sudo cp $webroot_dir/config/defaults.json $raspap_network || _install_status 1 "Unable to move defaults.json settings"
 
-        echo "Changing file ownership of $raspap_dir"
-        sudo chown -R $raspap_user:$raspap_user "$raspap_dir" || _install_status 1 "Unable to change file ownership for '$raspap_dir'"
+        echo "Changing file ownership of ${raspap_network}/defaults.json"
+        sudo chown $raspap_user:$raspap_user "$raspap_network"/defaults.json || _install_status 1 "Unable to change file ownership for defaults.json"
 
         echo "Checking for existence of /etc/dnsmasq.d"
         [ -d /etc/dnsmasq.d ] || sudo mkdir /etc/dnsmasq.d