Merge remote-tracking branch 'ben-grande/stat-dedup'

usr/bin/permission-hardener

@@ -6,9 +6,6 @@
 ## https://forums.whonix.org/t/disable-suid-binaries/7706
 ## https://forums.whonix.org/t/re-mount-home-and-other-with-noexec-and-nosuid-among-other-useful-mount-options-for-better-security/7707
 
-## TODO:
-## - unduplicate stat_output related source code
-
 set -o errexit -o nounset -o pipefail
 
 exit_code=0
@@ -17,6 +14,7 @@ dpkg_admindir_parameter_existing_mode="--admindir ${store_dir}/existing_mode"
 dpkg_admindir_parameter_new_mode="--admindir ${store_dir}/new_mode"
 delimiter="#permission-hardener-delimiter#"
 
+# shellcheck disable=SC2034
 log_level=notice
 # shellcheck disable=SC1091
 source /usr/libexec/helper-scripts/log_run_die.sh
@@ -53,6 +51,78 @@ make_store_dir(){
   mkdir --parents "${store_dir}/new_mode"
 }
 
+## Some tools may fail on newlines and even variable assignment to array may
+## fail if a variable that will be assigned to an array element contains
+## characters that are used as delimiters.
+block_newlines(){
+  local newline_variable newline_value
+  newline_variable="${1}"
+  newline_value="${2}"
+  ## dpkg-statoverride: error: path may not contain newlines
+  #if [[ "${newline_value}" == *$'\n'* ]]; then
+  if [[ "${newline_value}" != "${newline_value//$'\n'/NEWLINE}" ]]; then
+    log warn "Skipping ${newline_variable} that contains newlines: '${newline_value}'" >&2
+    return 1
+  fi
+}
+
+output_stat(){
+  local file_name
+  file_name="${1}"
+
+  if test -z "${file_name}"; then
+    log error "File name is empty. file_name: '${file_name}'" >&2
+    return 1
+  fi
+
+  block_newlines file "${file_name}"
+
+  declare -a arr
+  local file_name_from_stat existing_mode existing_owner existing_group stat_output stat_output_newlined
+
+  if ! stat_output="$(stat -c "%a${delimiter}%U${delimiter}%G${delimiter}%n${delimiter}" "${file_name}")"; then
+    log error "Failed to run 'stat' on file: '${file_name}'!" >&2
+    return 1
+  fi
+
+  stat_output_newlined="$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}")"
+  readarray -t arr <<< "${stat_output_newlined}"
+
+  if test "${#arr[@]}" = 0; then
+    log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2
+    return 1
+  fi
+
+  existing_mode="${arr[0]}"
+  existing_owner="${arr[1]}"
+  existing_group="${arr[2]}"
+  file_name_from_stat="${arr[3]}"
+
+  if [ ! "$file_name" = "$file_name_from_stat" ]; then
+    log error "\
+function ${FUNCNAME[1]}:
+File name is different from file name received from stat:
+File name '${file_name}'
+File name from stat: '${file_name_from_stat}'" >&2
+    return 1
+  fi
+
+  if test -z "${existing_mode}"; then
+    log error "Existing mode is empty. Stat output: '${stat_output}', line: '${line}'" >&2
+    return 1
+  fi
+  if test -z "${existing_owner}"; then
+    log error "Existing owner is empty. Stat output: '${stat_output}' | line: '${line}'" >&2
+    return 1
+  fi
+  if test -z "${existing_group}"; then
+    log error "Existing group is empty. Stat output: '${stat_output}', line: '${line}'" >&2
+    return 1
+  fi
+
+  (IFS=$'\n'; echo "${arr[*]}")
+}
+
 sanity_tests() {
   echo_wrapper_audit silent which \
     capsh getcap setcap stat find dpkg-statoverride getent xargs grep 1>/dev/null
@@ -73,60 +143,21 @@ add_nosuid_statoverride_entry() {
   done < <(find "${fso_to_process}" -perm /u=s,g=s -print0)
 
   local line
-  while IFS="" read -r -d "" line; do
+  while IFS="" read -r -d "" file_name; do
-    counter_actual="$((counter_actual + 1))"
+    counter_actual=$((counter_actual + 1))
 
-    local arr file_name file_name_from_stat existing_mode existing_owner existing_group stat_output stat_output_newlined
+    declare -a arr
-
+    local existing_mode existing_owner existing_group
-    file_name="${line}"
-
-    if test -z "${file_name}"; then
-      log error "File name is empty in line: '${line}'" >&2
-      continue
-    fi
-
-    stat_output=$(stat -c "%n${delimiter}%a${delimiter}%U${delimiter}%G${delimiter}" "${line}")
-    stat_output_newlined=$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}")
-    readarray -t arr <<< "${stat_output_newlined}"
 
+    readarray -t arr < <(output_stat "${file_name}")
+    ## Above command creates a subshell that cannot be returned.
     if test "${#arr[@]}" = 0; then
-      log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2
       continue
     fi
 
-    file_name_from_stat="${arr[0]}"
+    existing_mode="${arr[0]}"
-    existing_mode="${arr[1]}"
+    existing_owner="${arr[1]}"
-    existing_owner="${arr[2]}"
+    existing_group="${arr[2]}"
-    existing_group="${arr[3]}"
-
-    if [ ! "$file_name" = "$file_name_from_stat" ]; then
-      log error "\
-function add_nosuid_statoverride_entry:
-file_name is different from file_name_from_stat:
-line: '${line}'
-file_name '${file_name}'
-file_name_from_stat: '${file_name_from_stat}'" >&2
-      continue
-    fi
-
-    if test -z "${existing_mode}"; then
-      log error "Existing mode is empty in line: '${line}'" >&2
-      continue
-    fi
-    if test -z "${existing_owner}"; then
-      log error "Existing owner is empty in line: '${line}'" >&2
-      continue
-    fi
-    if test -z "${existing_group}"; then
-      log error "Existing group is empty in line: '${line}'" >&2
-      continue
-    fi
-
-    ## dpkg-statoverride: error: path may not contain newlines
-    if [[ "${file_name}" == *$'\n'* ]]; then
-      log warn "Skipping file name that contains newlines: '${file_name}'" >&2
-      continue
-    fi
 
     ## -h file      True if file is a symbolic Link.
     ## -u file      True if file has its set-user-id bit set.
@@ -143,27 +174,23 @@ file_name_from_stat: '${file_name_from_stat}'" >&2
       continue
     fi
 
-    local setuid setuid_output setsgid setsgid_output
+    local setuid setgid
     setuid=""
-    setuid_output=""
     if test -u "${file_name}"; then
       setuid=true
-      setuid_output="set-user-id"
     fi
-    setsgid=""
+    setgid=""
-    setsgid_output=""
     if test -g "${file_name}"; then
-      setsgid=true
+      setgid=true
-      setsgid_output="set-group-id"
     fi
 
-    local setuid_or_setsgid
+    local setuid_or_setgid
-    setuid_or_setsgid=""
+    setuid_or_setgid=""
-    if test "${setuid}" = "true" || test "${setsgid}" = "true"; then
+    if test "${setuid}" = "true" || test "${setgid}" = "true"; then
-      setuid_or_setsgid=true
+      setuid_or_setgid=true
     fi
-    if test -z "${setuid_or_setsgid}"; then
+    if test -z "${setuid_or_setgid}"; then
-      log info "Neither setuid nor setsgid. Skipping. file_name: '${file_name}'"
+      log info "Neither setuid nor setgid. Skipping. file_name: '${file_name}'"
       continue
     fi
 
@@ -224,7 +251,7 @@ file_name_from_stat: '${file_name_from_stat}'" >&2
 
     local clean_output_prefix clean_output
     clean_output_prefix="Managing (S|G)UID of line:"
-    clean_output="setuid='${setuid_output}' setgid='${setsgid_output}' existing_mode='${existing_mode}' new_mode='${new_mode}' file='${file_name}'"
+    clean_output="${setuid:+setuid='true'} ${setgid:+setgid='true'} existing_mode='${existing_mode}' new_mode='${new_mode}' file='${file_name}'"
     if test "${whitelists_disable_all:-}" = "true"; then
     log info "${clean_output_prefix} whitelists_disable_all=true ${clean_output}"
     elif test "${is_disable_whitelisted}" = "true"; then
@@ -335,7 +362,6 @@ set_file_perms() {
     local fso_without_trailing_slash
     fso_without_trailing_slash="${fso%/}"
 
-    ## TODO: test/add white spaces inside file name support
     declare -g disable_white_list exact_white_list match_white_list
     case "${mode_from_config}" in
       disablewhitelist)
@@ -393,54 +419,19 @@ set_file_perms() {
         mode_for_grep="${mode_from_config:1}"
       fi
 
-      local stat_output stat_output_newlined
+      declare -a arr
-      stat_output=""
+      local existing_mode existing_owner existing_group
-      if ! stat_output=$(stat -c "%n${delimiter}%a${delimiter}%U${delimiter}%G${delimiter}" "${fso_without_trailing_slash}"); then
-        log error "Failed to run 'stat' on file: '${fso_without_trailing_slash}'!" >&2
-        continue
-      fi
-      stat_output_newlined=$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}")
 
-      local arr file_name file_name_from_stat existing_mode existing_owner existing_group
-      readarray -t arr <<< "${stat_output_newlined}"
       file_name="${fso_without_trailing_slash}"
-
+      readarray -t arr < <(output_stat "${file_name}")
+      ## Above command creates a subshell that cannot be returned from.
       if test "${#arr[@]}" = 0; then
-        log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2
         continue
       fi
 
-      file_name_from_stat="${arr[0]}"
+      existing_mode="${arr[0]}"
-      existing_mode="${arr[1]}"
+      existing_owner="${arr[1]}"
-      existing_owner="${arr[2]}"
+      existing_group="${arr[2]}"
-      existing_group="${arr[3]}"
-
-    if [ ! "$file_name" = "$file_name_from_stat" ]; then
-      log error "\
-function set_file_perms:
-file_name is different from file_name_from_stat:
-line: '${line}'
-file_name '${file_name}'
-file_name_from_stat: '${file_name_from_stat}'" >&2
-      continue
-    fi
-
-      if test -z "${file_name}"; then
-        log error "File name is empty. Stat output: '${stat_output}', line: '${line}'" >&2
-        continue
-      fi
-      if test -z "${existing_mode}"; then
-        log error "Existing mode is empty. Stat output: '${stat_output}', line: '${line}'" >&2
-        continue
-      fi
-      if test -z "${existing_owner}"; then
-        log error "Existing owner is empty. Stat output: '${stat_output}' | line: '${line}'" >&2
-        continue
-      fi
-      if test -z "${existing_group}"; then
-        log error "Existing group is empty. Stat output: '${stat_output}', line: '${line}'" >&2
-        continue
-      fi
 
       ## Check there is an entry for the fso.
       ##
@@ -558,9 +549,9 @@ parse_config_folder() {
   ## 'grep' exits after the first match in this case causing 'getent' to
   ## receive SIGPIPE, which then fails the pipeline since 'set -o pipefail' is
   ## set for this script.
-  passwd_file_contents_temp=$(getent passwd)
+  passwd_file_contents_temp="$(getent passwd)"
   echo "${passwd_file_contents_temp}" | tee "${store_dir}/private/passwd" >/dev/null
-  group_file_contents_temp=$(getent group)
+  group_file_contents_temp="$(getent group)"
   echo "${group_file_contents_temp}" | tee "${store_dir}/private/group" >/dev/null
 
   #passwd_file_contents="$(cat "${store_dir}/private/passwd")"