(fix): Hardware Info Root Sudoers entry was possible to hijack

install_linux.sh

@@ -124,8 +124,9 @@ if [[ $activate_scenario_runner == true ]] ; then
                 sudo apt-get install -y freeipmi-tools ipmitool
             fi
             print_message "Adding IPMI to sudoers file"
-            check_file_permissions "/usr/sbin/ipmi-dcmi"
-            echo "${USER} ALL=(ALL) NOPASSWD:/usr/sbin/ipmi-dcmi --get-system-power-statistics" | sudo tee /etc/sudoers.d/green-coding-ipmi-get-machine-energy-stat
+            ipmi_dcmi_path=$(readlink -f "/usr/sbin/ipmi-dcmi")
+            check_file_permissions $ipmi_dcmi_path
+            echo "${USER} ALL=(ALL) NOPASSWD:${ipmi_dcmi_path} --get-system-power-statistics" | sudo tee /etc/sudoers.d/green-coding-ipmi-get-machine-energy-stat
             sudo chmod 500 /etc/sudoers.d/green-coding-ipmi-get-machine-energy-stat
             # remove old file name
             sudo rm -f /etc/sudoers.d/ipmi_get_machine_energy_stat

install_mac.sh

@@ -30,11 +30,13 @@ if [[ $activate_scenario_runner == true ]] ; then
     build_binaries
 
     print_message "Adding powermetrics to sudoers file"
-    check_file_permissions "/usr/bin/powermetrics"
-    check_file_permissions "/usr/bin/killall"
-    echo "${USER} ALL=(ALL) NOPASSWD:/usr/bin/powermetrics" | sudo tee /etc/sudoers.d/green_coding_powermetrics
-    echo "${USER} ALL=(ALL) NOPASSWD:/usr/bin/killall powermetrics" | sudo tee /etc/sudoers.d/green_coding_kill_powermetrics
-    echo "${USER} ALL=(ALL) NOPASSWD:/usr/bin/killall -9 powermetrics" | sudo tee /etc/sudoers.d/green_coding_kill_powermetrics_sigkill
+    powermetrics_path=$(readlink -f "/usr/bin/powermetrics")
+    killall_path=$(readlink -f "/usr/bin/killall")
+    check_file_permissions $powermetrics_path
+    check_file_permissions $killall_path
+    echo "${USER} ALL=(ALL) NOPASSWD:${powermetrics_path}" | sudo tee /etc/sudoers.d/green_coding_powermetrics
+    echo "${USER} ALL=(ALL) NOPASSWD:${killall_path} powermetrics" | sudo tee /etc/sudoers.d/green_coding_kill_powermetrics
+    echo "${USER} ALL=(ALL) NOPASSWD:${killall_path} -9 powermetrics" | sudo tee /etc/sudoers.d/green_coding_kill_powermetrics_sigkill
 
 fi
 

lib/install_shared.sh

@@ -61,49 +61,84 @@ function check_python_version() {
 function check_file_permissions() {
     local file=$1
 
-    # Check if the file exists
     if [ ! -e "$file" ]; then
         echo "File '$file' does not exist."
         return 1
     fi
 
-    # Check if the file is owned by root
+    if [ -L "$file" ]; then
+        echo "File '$file' is a symbolic link. This is not allowed."
+        return 1
+    fi
+
+    # Determine stat commands based on OS
+    local owner file_mode dir_owner dir_mode
+    local dir=$(dirname "$file")
+
     if [[ $(uname) == "Darwin" ]]; then
-        if [ "$(stat -f %Su "$file")" != "root" ]; then
-            echo "File '$file' is not owned by root."
+        if ls -lde "$file" | grep -q " 0:"; then
+            echo "File '$file' has an ACL. Unsafe!"
             return 1
         fi
+        if ls -lde "$dir" | grep -q " 0:"; then
+            echo "Directory '$dir' has an ACL. Unsafe!"
+            return 1
+        fi
+        # Numeric mode
+        owner=$(stat -f %Su "$file")
+        file_mode=$(stat -f %Lp "$file")   # outputs octal like 100400
+        dir_owner=$(stat -f %Su "$dir")
+        dir_mode=$(stat -f %Lp "$dir")     # outputs octal like 40755
     else
-        if [ "$(stat -c %U "$file")" != "root" ]; then
-            echo "File '$file' is not owned by root."
+        owner=$(stat -c %U "$file")
+        file_mode=$(stat -c %a "$file")       # numeric mode, e.g., 400
+        dir_owner=$(stat -c %U "$dir")
+        dir_mode=$(stat -c %a "$dir")         # numeric mode, e.g., 755
+        # Check ACLs
+        if getfacl -c "$file" 2>/dev/null | awk '!/^#|^user::|^group::|^other::/' | grep -q; then
+            echo "File '$file' has an ACL. Unsafe!"
             return 1
         fi
+        if getfacl -c "$dir" 2>/dev/null | awk '!/^#|^user::|^group::|^other::/' | grep -q; then
+            echo "Directory '$dir' has an ACL. Unsafe!"
+            return 1
+        fi
+
     fi
 
-        # Check if the file is owned by root
+    # Check ownership
+    if [[ "$owner" != "root" ]]; then
+        echo "File '$file' is not owned by root."
+        return 1
+    fi
+    if [[ "$dir_owner" != "root" ]]; then
+        echo "Parent directory '$dir' is not owned by root."
+        return 1
+    fi
 
-    # Check if the file permissions are read-only for group and others using regex
+    local file_perm_numeric=$((10#${file_mode: -3}))  # last 3 digits
+    local file_group=$(( (file_perm_numeric / 10 % 10) ))
+    local file_other=$(( file_perm_numeric % 10 ))
 
-    if [[ $(uname) == "Darwin" ]]; then
-        local permissions=$(stat -f %Sp "$file")
-    else
-        local permissions=$(stat -c %A "$file") # Linux
+    if (( file_group & 2 )) || (( file_other & 2 )); then
+        echo "File '$file' is writable by group or others. Unsafe!"
+        return 1
     fi
+    # Check directory permissions: group/others must NOT have write (0x2 in octal)
+    local dir_perm_numeric=$((10#${dir_mode: -3}))
+    local dir_group=$(( (dir_perm_numeric / 10 % 10) ))
+    local dir_other=$(( dir_perm_numeric % 10 ))
 
-    if [ -L "$file" ]; then
-        echo "File '$file' is a symbolic link. Following ..."
-        check_file_permissions $(readlink -f $file)
-        return $?
-    elif [[ ! $permissions =~ ^-r..r-.r-.$ ]]; then
-        echo "File '$file' is not read-only for group and others or not a regular file"
+    if (( dir_group & 2 )) || (( dir_other & 2 )); then
+        echo "Parent directory '$dir' is writable by group or others. Unsafe!"
         return 1
     fi
 
-    echo "File $file is save to create sudoers entry for"
-
+    echo "File '$file' and its parent directory are secure."
     return 0
 }
 
+
 function copy_backup() {
     local file=$1
     local example_file="${file}.example"
@@ -260,17 +295,20 @@ function setup_python() {
     print_message "Setting GMT in include path for python via .pth file"
     find venv -type d -name "site-packages" -exec sh -c 'echo $PWD > "$0/gmt-lib.pth"' {} \;
 
-    print_message "Adding python3 lib.hardware_info_root to sudoers file"
-    check_file_permissions "/usr/bin/python3"
-    # Please note the -m as here we will later call python3 without venv. It must understand the .lib imports
-    # and not depend on venv installed packages
-    echo "${USER} ALL=(ALL) NOPASSWD:/usr/bin/python3 -m lib.hardware_info_root" | sudo tee /etc/sudoers.d/green-coding-hardware-info
-    echo "${USER} ALL=(ALL) NOPASSWD:/usr/bin/python3 -m lib.hardware_info_root --read-rapl-energy-filtering" | sudo tee -a /etc/sudoers.d/green-coding-hardware-info
+    print_message "Adding python3 hardware_info_root.py to sudoers file"
+    local python_path=$(readlink -f "/usr/bin/python3")
+    check_file_permissions $python_path
+    # Please note the -m as here we will later call python3 without venv.
+    # It must only use python root installed packages and no venv packages
+    # furthermore it may only use an absolute path
+    local hardware_info_root_path="$(pwd)/lib/hardware_info_root.py"
+    echo "${USER} ALL=(ALL) NOPASSWD:${python_path} ${hardware_info_root_path}" | sudo tee /etc/sudoers.d/green-coding-hardware-info
+    echo "${USER} ALL=(ALL) NOPASSWD:${python_path} ${hardware_info_root_path} --read-rapl-energy-filtering" | sudo tee -a /etc/sudoers.d/green-coding-hardware-info
     sudo chmod 500 /etc/sudoers.d/green-coding-hardware-info
     # remove old file name
     sudo rm -f /etc/sudoers.d/green_coding_hardware_info
 
-    print_message "Setting the hardare hardware_info to be owned by root"
+    print_message "Making hardware_info_root.py to be owned by root"
     sudo cp -f $PWD/lib/hardware_info_root_original.py $PWD/lib/hardware_info_root.py
     sudo chown root:$(id -gn root) $PWD/lib/hardware_info_root.py
     sudo chmod 755 $PWD/lib/hardware_info_root.py

lib/scenario_runner.py

@@ -26,7 +26,8 @@
 from concurrent.futures import ThreadPoolExecutor
 from energy_dependency_inspector import resolve_docker_dependencies_as_dict
 
-GMT_ROOT_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), '../')
+CURRENT_DIR = os.path.dirname(os.path.realpath(__file__))
+GMT_ROOT_DIR = os.path.join(CURRENT_DIR, '../')
 
 from lib import utils
 from lib import process_helpers
@@ -794,7 +795,7 @@ def _initialize_run(self):
         machine_specs = hardware_info.get_default_values()
 
         if len(hardware_info_root.get_root_list()) > 0:
-            ps = subprocess.run(['sudo', '/usr/bin/python3', '-m', 'lib.hardware_info_root'], stdout=subprocess.PIPE, cwd=GMT_ROOT_DIR, check=True, encoding='UTF-8', errors='replace')
+            ps = subprocess.run(['sudo', '/usr/bin/python3', os.path.realpath(os.path.join(CURRENT_DIR, 'hardware_info_root.py'))], stdout=subprocess.PIPE, cwd=GMT_ROOT_DIR, check=True, encoding='UTF-8', errors='replace')
             machine_specs_root = json.loads(ps.stdout)
             machine_specs.update(machine_specs_root)
 

lib/utils.py

@@ -213,7 +213,7 @@ def get_architecture():
 
 
 def is_rapl_energy_filtering_deactivated():
-    result = subprocess.run(['sudo', 'python3', '-m', 'lib.hardware_info_root', '--read-rapl-energy-filtering'],
+    result = subprocess.run(['sudo', '/usr/bin/python3', os.path.realpath(os.path.join(CURRENT_DIR, 'hardware_info_root.py')), '--read-rapl-energy-filtering'],
                             stdout=subprocess.PIPE,
                             stderr=subprocess.PIPE,
                             cwd=os.path.abspath(os.path.join(CURRENT_DIR, '..')),