Merge pull request #1357 from YunoHost/safer_ynh_secure_remove

Safer, clearer ynh_secure_remove

data/helpers.d/utils

@@ -724,6 +724,28 @@ properly with chmod/chown."
     echo $TMP_DIR
 }
 
+_acceptable_path_to_delete() {
+    local file=$1
+
+    local forbidden_paths=$(ls -d / /* /{var,home,usr}/* /etc/{default,sudoers.d,yunohost,cron*})
+
+    # Legacy : A couple apps still have data in /home/$app ...
+    if [[ -n "$app" ]]
+    then
+        forbidden_paths=$(echo "$forbidden_paths" | grep -v "/home/$app")
+    fi
+
+    # Use realpath to normalize the path ..
+    # i.e convert ///foo//bar//..///baz//// to /foo/baz
+    file=$(realpath --no-symlinks "$file")
+    if [ -z "$file" ] || grep -q -x -F "$file" <<< "$forbidden_paths"; then
+        return 1
+    else
+        return 0
+    fi
+}
+
+
 # Remove a file or a directory securely
 #
 # usage: ynh_secure_remove --file=path_to_remove
@@ -739,31 +761,18 @@ ynh_secure_remove () {
     ynh_handle_getopts_args "$@"
     set +o xtrace # set +x
 
-    local forbidden_path=" \
+    if [ $# -ge 2 ]; then
-    /var/www \
-    /home/yunohost.app"
-
-    if [ $# -ge 2 ]
-    then
         ynh_print_warn --message="/!\ Packager ! You provided more than one argument to ynh_secure_remove but it will be ignored... Use this helper with one argument at time."
     fi
 
-    if [[ -z "$file" ]]
+    if [[ -z "$file" ]]; then
-    then
         ynh_print_warn --message="ynh_secure_remove called with empty argument, ignoring."
-    elif [[ "$forbidden_path" =~ "$file" \
+    elif [[ ! -e $file ]]; then
-        # Match all paths or subpaths in $forbidden_path
-        || "$file" =~ ^/[[:alnum:]]+$ \
-        # Match all first level paths from / (Like /var, /root, etc...)
-        || "${file:${#file}-1}" = "/" ]]
-        # Match if the path finishes by /. Because it seems there is an empty variable
-    then
-        ynh_print_warn --message="Not deleting '$file' because it is not an acceptable path to delete."
-    elif [ -e "$file" ]
-    then
-        rm --recursive "$file"
-    else
         ynh_print_info --message="'$file' wasn't deleted because it doesn't exist."
+    elif ! _acceptable_path_to_delete "$file"; then
+        ynh_print_warn --message="Not deleting '$file' because it is not an acceptable path to delete."
+    else
+        rm --recursive "$file"
     fi
 
     set -o xtrace # set -x

tests/test_helpers.d/ynhtest_secure_remove.sh

@@ -0,0 +1,71 @@
+ynhtest_acceptable_path_to_delete() {
+    
+    mkdir -p /home/someuser
+    mkdir -p /home/$app
+    mkdir -p /home/yunohost.app/$app
+    mkdir -p /var/www/$app
+    touch /var/www/$app/bar
+    touch /etc/cron.d/$app
+
+    ! _acceptable_path_to_delete /
+    ! _acceptable_path_to_delete ////
+    ! _acceptable_path_to_delete "    ////   "
+    ! _acceptable_path_to_delete /var
+    ! _acceptable_path_to_delete /var/www
+    ! _acceptable_path_to_delete /var/cache
+    ! _acceptable_path_to_delete /usr
+    ! _acceptable_path_to_delete /usr/bin
+    ! _acceptable_path_to_delete /home
+    ! _acceptable_path_to_delete /home/yunohost.backup
+    ! _acceptable_path_to_delete /home/yunohost.app
+    ! _acceptable_path_to_delete /home/yunohost.app/
+    ! _acceptable_path_to_delete ///home///yunohost.app///
+    ! _acceptable_path_to_delete /home/yunohost.app/$app/..
+    ! _acceptable_path_to_delete ///home///yunohost.app///$app///..//
+    ! _acceptable_path_to_delete /home/yunohost.app/../$app/..
+    ! _acceptable_path_to_delete /home/someuser
+    ! _acceptable_path_to_delete /home/yunohost.app//../../$app
+    ! _acceptable_path_to_delete "  /home/yunohost.app///  "
+    ! _acceptable_path_to_delete /etc/cron.d/
+    ! _acceptable_path_to_delete /etc/yunohost/
+
+    _acceptable_path_to_delete /home/yunohost.app/$app
+    _acceptable_path_to_delete /home/yunohost.app/$app/bar
+    _acceptable_path_to_delete /etc/cron.d/$app
+    _acceptable_path_to_delete /var/www/$app/bar
+    _acceptable_path_to_delete /var/www/$app
+
+    rm /var/www/$app/bar
+    rm /etc/cron.d/$app
+    rmdir /home/yunohost.app/$app
+    rmdir /home/$app
+    rmdir /home/someuser
+    rmdir /var/www/$app
+}
+
+ynhtest_secure_remove() {
+
+    mkdir -p /home/someuser
+    mkdir -p /home/yunohost.app/$app
+    mkdir -p /var/www/$app
+    mkdir -p /var/whatever
+    touch /var/www/$app/bar
+    touch /etc/cron.d/$app
+
+    ! ynh_secure_remove --file="/home/someuser"
+    ! ynh_secure_remove --file="/home/yunohost.app/"
+    ! ynh_secure_remove --file="/var/whatever"
+    ynh_secure_remove --file="/home/yunohost.app/$app"
+    ynh_secure_remove --file="/var/www/$app"
+    ynh_secure_remove --file="/etc/cron.d/$app"
+
+    test -e /home/someuser
+    test -e /home/yunohost.app
+    test -e /var/whatever
+    ! test -e /home/yunohost.app/$app
+    ! test -e /var/www/$app
+    ! test -e /etc/cron.d/$app
+
+    rmdir /home/someuser
+    rmdir /var/whatever
+}