Install script safety/security concerns: unsafe `$PREFIX`, symlink handling, non-portable commands

[peppapig450]

Description

The current install.sh script works for trivial cases (e.g., default environments), but it has security and portability issues that could lead to accidental data loss or be exploited when run as root,

Problems

1. $PREFIX not validated

   • If a user sets PREFIX=/, or if an environment variable PREFIX is set, the script will run:

     rm -rf /share/anki /bin/anki
     

     which could delete critical system directories. Similar risks exist if $PREFIX is /etc, /root, or any other sensitive path.

2. No check for symlinks

   • The script does rm -rf "$PREFIX/share/anki" without verifying whether it’s a symlink.
   • An attacker or misconfiguration could make $PREFIX/share/anki a symlink to / or another sensitive directory, leading to arbitrary deletion or overwriting.

3. Lack of -- in command invocations

   • Commands like mv, cp, and rm should use -- to separate options from file names.
   • Without it, files starting with - (e.g., -rf) could be misinterpreted as options.

4. Use of non-portable options

   • mv -Z is not POSIX and may fail on systems without SELinux, breaking the script.
   • Relying on non-standard flags reduces portability and reliability.

5. Use of cp/mv instead of install

   • The install command is safer and more predictable for setting permissions, ownership, and modes, rather than relying on cp and mv.

Recommendations

• Validate $PREFIX

  case "$PREFIX" in
    ""|"/"|"/root"|"/etc") 
        echo "Unsafe PREFIX: $PREFIX"
        exit 1
        ;;
  esac
  
  

• Check for symlinks before removing directories and/or use command options that do not dereference symlinks (most are non POSIX)

  if [ -L "$PREFIX/share/anki" ]; then
    echo "Refusing to operate on symlinked $PREFIX/share/anki"
    exit 1
  fi
  

• Use -- consistently

   rm -rf -- "$PREFIX/share/anki" "$PREFIX/bin/anki"
   cp -av --no-preserve=owner,context -- * .python-version "$PREFIX/share/anki/"
   mv -- anki.png "$PREFIX/share/pixmaps/"
  

• Replace cp/mv with install:

   install -Dm755 anki "$PREFIX/bin/anki"
   install -Dm644 anki.png "$PREFIX/share/pixmaps/anki.png"
   install -Dm644 anki.desktop "$PREFIX/share/applications/anki.desktop"
   install -Dm644 anki.1 "$PREFIX/share/man/man1/anki.1"
  

• Avoid non-portable flags (-Z)

  • Drop -Z or guard it with a conditional that checks whether SELinux is in use.

• Consider user-local installs by default

  • Defaulting to PREFIX=$HOME/.local is safer than /usr/local, unless the user explicitly overrides it.

---

I’d be happy to propose a safer, more portable rewrite of install.sh once I know which Bash version (or POSIX shell baseline) this project targets. For example:

If Bash ≥ 4 is acceptable, we can use stricter idioms (set -o pipefail, array handling, etc.).

If POSIX sh compatibility is required, I’ll stick to portable constructs and avoid Bashisms.

This would let me submit a PR that hardens the script against the issues listed above (validated $PREFIX, safer rm, no -Z, consistent use of install, etc.) while staying compatible with your supported environments.

[GithubAnon0000]

I'd say create a small PR with one of the changes you propose. PRs do get attention, issues might fall below the radar.

Personally, I'd love to see hardened shell scripts.

[GithubAnon0000]

Oh and, btw: Probably it would be better to follow https://github.com/ankitects/anki/blob/main/SECURITY.md regarding security related issues. Dae will see it if you contact him that way.

[DennisDyallo]

I was just experiencing trouble with the install script, as I am on Arch Linux, where /usr/ is locked and requires root.

I rewrote the script to use $HOME instead and now I am happy. Thanks for bringing it up @peppapig450