Remove pihole-FTL.port file (#4945)

6 files changed, 42 insertions, 63 deletions

diff --git a/advanced/Scripts/chronometer.sh b/advanced/Scripts/chronometer.sh
index af007994..d69a56d3 100755
--- a/advanced/Scripts/chronometer.sh
+++ b/advanced/Scripts/chronometer.sh
@@ -14,7 +14,9 @@ LC_NUMERIC=C
 # Retrieve stats from FTL engine
 pihole-FTL() {
     local ftl_port LINE
-    ftl_port=$(cat /run/pihole-FTL.port 2> /dev/null)
+    # shellcheck disable=SC1091
+    . /opt/pihole/utils.sh
+    ftl_port=$(getFTLAPIPort)
     if [[ -n "$ftl_port" ]]; then
         # Open connection to FTL
         exec 3<>"/dev/tcp/127.0.0.1/$ftl_port"
diff --git a/advanced/Scripts/piholeDebug.sh b/advanced/Scripts/piholeDebug.sh
index 91e16850..dbf56709 100755
--- a/advanced/Scripts/piholeDebug.sh
+++ b/advanced/Scripts/piholeDebug.sh
@@ -126,7 +126,6 @@ PIHOLE_COMMAND="${BIN_DIRECTORY}/pihole"
 PIHOLE_COLTABLE_FILE="${BIN_DIRECTORY}/COL_TABLE"
 
 FTL_PID="${RUN_DIRECTORY}/pihole-FTL.pid"
-FTL_PORT="${RUN_DIRECTORY}/pihole-FTL.port"
 
 PIHOLE_LOG="${LOG_DIRECTORY}/pihole.log"
 PIHOLE_LOG_GZIPS="${LOG_DIRECTORY}/pihole.log.[0-9].*"
@@ -155,7 +154,6 @@ REQUIRED_FILES=("${PIHOLE_CRON_FILE}"
 "${PIHOLE_COMMAND}"
 "${PIHOLE_COLTABLE_FILE}"
 "${FTL_PID}"
-"${FTL_PORT}"
 "${PIHOLE_LOG}"
 "${PIHOLE_LOG_GZIPS}"
 "${PIHOLE_DEBUG_LOG}"
diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh
index a9e05692..1174fa62 100755
--- a/advanced/Scripts/utils.sh
+++ b/advanced/Scripts/utils.sh
@@ -32,8 +32,8 @@ addOrEditKeyValPair() {
   local value="${3}"
 
   if grep -q "^${key}=" "${file}"; then
-      # Key already exists in file, modify the value
-      sed -i "/^${key}=/c\\${key}=${value}" "${file}"
+    # Key already exists in file, modify the value
+    sed -i "/^${key}=/c\\${key}=${value}" "${file}"
   else
     # Key does not already exist, add it and it's value
     echo "${key}=${value}" >> "${file}"
@@ -52,8 +52,8 @@ addKey(){
   local key="${2}"
 
   if ! grep -q "^${key}" "${file}"; then
-      # Key does not exist, add it.
-      echo "${key}" >> "${file}"
+    # Key does not exist, add it.
+    echo "${key}" >> "${file}"
   fi
 }
 
@@ -70,47 +70,27 @@ removeKey() {
   sed -i "/^${key}/d" "${file}"
 }
 
-#######################
-# returns path of FTL's port file
-#######################
-getFTLAPIPortFile() {
-    local FTLCONFFILE="/etc/pihole/pihole-FTL.conf"
-    local DEFAULT_PORT_FILE="/run/pihole-FTL.port"
-    local FTL_APIPORT_FILE
-
-    if [ -s "${FTLCONFFILE}" ]; then
-        # if PORTFILE is not set in pihole-FTL.conf, use the default path
-        FTL_APIPORT_FILE="$({ grep '^PORTFILE=' "${FTLCONFFILE}" || echo "${DEFAULT_PORT_FILE}"; } | cut -d'=' -f2-)"
-    else
-        # if there is no pihole-FTL.conf, use the default path
-        FTL_APIPORT_FILE="${DEFAULT_PORT_FILE}"
-    fi
-
-      echo "${FTL_APIPORT_FILE}"
-}
 
-
-#######################
-# returns FTL's current telnet API port based on the content of the pihole-FTL.port file
-#
-# Takes one argument: path to pihole-FTL.port
-# Example getFTLAPIPort "/run/pihole-FTL.port"
 #######################
+# returns FTL's current telnet API port based on the setting in /etc/pihole-FTL.conf
+########################
 getFTLAPIPort(){
-    local PORTFILE="${1}"
+    local FTLCONFFILE="/etc/pihole/pihole-FTL.conf"
     local DEFAULT_FTL_PORT=4711
     local ftl_api_port
 
-    if [ -s "$PORTFILE" ]; then
-        # -s: FILE exists and has a size greater than zero
-        ftl_api_port=$(cat "${PORTFILE}")
-        # Exploit prevention: unset the variable if there is malicious content
-        # Verify that the value read from the file is numeric
-        expr "$ftl_api_port" : "[^[:digit:]]" > /dev/null && unset ftl_api_port
+    if [ -s "$FTLCONFFILE" ]; then
+        # if FTLPORT is not set in pihole-FTL.conf, use the default port
+        ftl_api_port="$({ grep '^FTLPORT=' "${FTLCONFFILE}" || echo "${DEFAULT_FTL_PORT}"; } | cut -d'=' -f2-)"
+        # Exploit prevention: set the port to the default port if there is malicious (non-numeric)
+        # content set in pihole-FTL.conf
+        expr "${ftl_api_port}" : "[^[:digit:]]" > /dev/null && ftl_api_port="${DEFAULT_FTL_PORT}"
+    else
+        # if there is no pihole-FTL.conf, use the default port
+        ftl_api_port="${DEFAULT_FTL_PORT}"
     fi
 
-    # echo the port found in the portfile or default to the default port
-    echo "${ftl_api_port:=$DEFAULT_FTL_PORT}"
+    echo "${ftl_api_port}"
 }
 
 #######################
diff --git a/advanced/Templates/pihole-FTL.service b/advanced/Templates/pihole-FTL.service
index 46e5c1f2..bc1b1d20 100644
--- a/advanced/Templates/pihole-FTL.service
+++ b/advanced/Templates/pihole-FTL.service
@@ -9,7 +9,7 @@
 # Description:       Enable service provided by pihole-FTL daemon
 ### END INIT INFO
 
-#source utils.sh for getFTLPIDFile(), getFTLPID (), getFTLAPIPortFile()
+#source utils.sh for getFTLPIDFile(), getFTLPID ()
 PI_HOLE_SCRIPT_DIR="/opt/pihole"
 utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
 . "${utilsfile}"
@@ -31,7 +31,6 @@ start() {
     # Touch files to ensure they exist (create if non-existing, preserve if existing)
     mkdir -pm 0755 /run/pihole /var/log/pihole
     [ ! -f "${FTL_PID_FILE}" ] && install -D -m 644 -o pihole -g pihole /dev/null "${FTL_PID_FILE}"
-    [ ! -f "${FTL_PORT_FILE}" ] && install -D -m 644 -o pihole -g pihole /dev/null "${FTL_PORT_FILE}"
     [ ! -f /var/log/pihole/FTL.log ] && install -m 644 -o pihole -g pihole /dev/null /var/log/pihole/FTL.log
     [ ! -f /var/log/pihole/pihole.log ] && install -m 640 -o pihole -g pihole /dev/null /var/log/pihole/pihole.log
     [ ! -f /etc/pihole/dhcp.leases ] && install -m 644 -o pihole -g pihole /dev/null /etc/pihole/dhcp.leases
@@ -91,7 +90,7 @@ stop() {
     echo "Not running"
   fi
   # Cleanup
-  rm -f /run/pihole/FTL.sock /dev/shm/FTL-* "${FTL_PID_FILE}" "${FTL_PORT_FILE}"
+  rm -f /run/pihole/FTL.sock /dev/shm/FTL-* "${FTL_PID_FILE}"
   echo
 }
 
@@ -111,7 +110,6 @@ status() {
 
 # Get file paths
 FTL_PID_FILE="$(getFTLPIDFile)"
-FTL_PORT_FILE="$(getFTLAPIPortFile)"
 
 # Get FTL's current PID
 FTL_PID="$(getFTLPID ${FTL_PID_FILE})"
diff --git a/pihole b/pihole
index 1047d152..aad83451 100755
--- a/pihole
+++ b/pihole
@@ -303,14 +303,13 @@ analyze_ports() {
 
 statusFunc() {
     # Determine if there is pihole-FTL service is listening
-    local pid port ftl_api_port ftl_pid_file ftl_apiport_file
+    local pid port ftl_api_port ftl_pid_file
 
     ftl_pid_file="$(getFTLPIDFile)"
 
     pid="$(getFTLPID ${ftl_pid_file})"
 
-    ftl_apiport_file="${getFTLAPIPortFile}"
-    ftl_api_port="$(getFTLAPIPort ${ftl_apiport_file})"
+    ftl_api_port="$(getFTLAPIPort)"
     if [[ "$pid" -eq "-1" ]]; then
         case "${1}" in
             "web") echo "-1";;
diff --git a/test/test_any_utils.py b/test/test_any_utils.py
index a2604dc2..5b4075d9 100644
--- a/test/test_any_utils.py
+++ b/test/test_any_utils.py
@@ -62,47 +62,49 @@ def test_key_removal_works(host):
     assert expected_stdout == output.stdout
 
 
-def test_getFTLAPIPortFile_default(host):
-    """Confirms getFTLAPIPortFile returns the default API port file path"""
+def test_getFTLAPIPort_default(host):
+    """Confirms getFTLAPIPort returns the default API port"""
     output = host.run(
         """
     source /opt/pihole/utils.sh
-    getFTLAPIPortFile
+    getFTLAPIPort
     """
     )
-    expected_stdout = "/run/pihole-FTL.port\n"
+    expected_stdout = "4711\n"
     assert expected_stdout == output.stdout
 
 
-def test_getFTLAPIPort_default(host):
-    """Confirms getFTLAPIPort returns the default API port"""
+def test_getFTLAPIPort_custom(host):
+    """Confirms getFTLAPIPort returns a custom API port"""
+    host.run(
+        """
+    echo "FTLPORT=1234" > /etc/pihole/pihole-FTL.conf
+    """
+    )
     output = host.run(
         """
     source /opt/pihole/utils.sh
-    getFTLAPIPort "/run/pihole-FTL.port"
+    getFTLAPIPort
     """
     )
-    expected_stdout = "4711\n"
+    expected_stdout = "1234\n"
     assert expected_stdout == output.stdout
 
 
-def test_getFTLAPIPortFile_and_getFTLAPIPort_custom(host):
-    """Confirms getFTLAPIPort returns a custom API port in a custom PORTFILE location"""
+def test_getFTLAPIPort_malicious(host):
+    """Confirms getFTLAPIPort returns 4711 if the setting in pihole-FTL.conf contains non-digits"""
     host.run(
         """
-    tmpfile=$(mktemp)
-    echo "PORTFILE=${tmpfile}" > /etc/pihole/pihole-FTL.conf
-    echo "1234" > ${tmpfile}
+    echo "FTLPORT=*$ssdfsd" > /etc/pihole/pihole-FTL.conf
     """
     )
     output = host.run(
         """
     source /opt/pihole/utils.sh
-    FTL_API_PORT_FILE=$(getFTLAPIPortFile)
-    getFTLAPIPort "${FTL_API_PORT_FILE}"
+    getFTLAPIPort
     """
     )
-    expected_stdout = "1234\n"
+    expected_stdout = "4711\n"
     assert expected_stdout == output.stdout