Sicherheitshaertung: Injection-Schutz, atomares Locking, Pfad-Validierung

- load_config blockiert Shell-Operatoren (;|&`) in .env-Werten
- validate_path prueft Sonderzeichen und Path-Traversal in Pfad-Variablen
- validate_config prüft DARKTABLE_BIN-basename und ruft validate_path auf
- Lockdir-Trap erst nach erfolgreicher Lock-Akquisition registriert
  (verhindert dass externer Lockdir bei gescheitertem Lock entfernt wird)
- uninstall.sh nutzt rmdir statt rm -rf fuer Lockdir
- security.bats mit 10 Tests fuer alle Sicherheitsanforderungen

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

scripts/darktable_common.sh

@@ -11,6 +11,20 @@ load_config() {
         echo "Vorlage kopieren mit: cp .env.example $env_file" >&2
         exit 1
     fi
+
+    # Berechtigungen pruefen: .env darf nicht world-readable sein
+    local perms
+    perms=$(stat -c '%a' "$env_file" 2>/dev/null || stat -f '%A' "$env_file" 2>/dev/null)
+    if [[ "${perms: -1}" != "0" ]]; then
+        echo "Warnung: $env_file ist world-readable. Empfehlung: chmod 600 $env_file" >&2
+    fi
+
+    # Zeilen mit Shell-Operatoren abweisen (Kommentare und Leerzeilen ignorieren)
+    if grep -vE '^\s*#|^\s*$' "$env_file" | grep -qE '[;|&`]'; then
+        echo "Fehler: $env_file enthaelt unerlaubte Zeichen (; | & \`). Bitte pruefen." >&2
+        exit 1
+    fi
+
     # shellcheck source=/dev/null
     . "$env_file"
 }
@@ -23,6 +37,15 @@ require_var() {
     fi
 }
 
+validate_path() {
+    local var_name="$1" value="${!1:-}"
+    # Pfade duerfen keine Shell-Sonderzeichen oder Path-Traversal enthalten
+    if echo "$value" | grep -qE "['\";|&\`\$()\\\\]" || [[ "$value" == *".."* ]]; then
+        echo "Fehler: '$var_name' enthaelt unerlaubte Zeichen: $value" >&2
+        exit 1
+    fi
+}
+
 validate_config() {
     require_var SERVER_IP
     require_var SERVER_USER
@@ -33,6 +56,15 @@ validate_config() {
     require_var LOCAL_PHOTO_DIR
     require_var SYNC_BIN
     require_var DARKTABLE_BIN
+
+    validate_path SERVER_DB_DIR
+    validate_path SERVER_PHOTO_DIR
+
+    # DARKTABLE_BIN: basename muss 'darktable' sein
+    if [[ "$(basename "$DARKTABLE_BIN")" != "darktable" ]]; then
+        echo "Fehler: DARKTABLE_BIN muss auf 'darktable' zeigen, nicht auf '$(basename "$DARKTABLE_BIN")'." >&2
+        exit 1
+    fi
 }
 
 check_dependency() {

scripts/darktable_sync.sh

@@ -24,17 +24,15 @@ count_synced_files() {
     echo "$count"
 }
 
-SCRIPT_NAME="$(basename "$0")"
+LOCKDIR="$CONFIG_DIR/sync.lock"
-LOCKFILE="/tmp/${SCRIPT_NAME}.lock"
+TMPFILES=()
 
-if [ -e "$LOCKFILE" ]; then
+# Atomares Locking per mkdir (verhindert Race Condition und Symlink-Attacken)
-    echo "Script laeuft bereits oder Lockfile loeschen: $LOCKFILE"
+if ! mkdir "$LOCKDIR" 2>/dev/null; then
+    echo "Script laeuft bereits oder Lockdir loeschen: $LOCKDIR"
     exit 1
 fi
-
+trap 'rm -f "${TMPFILES[@]}"; rmdir "$LOCKDIR" 2>/dev/null || true' EXIT
-touch "$LOCKFILE"
-TMPFILES=("$LOCKFILE")
-trap 'rm -f "${TMPFILES[@]}"' EXIT
 
 SHOW_NOTIFY_START_STOP=false
 if [[ "${1:-}" == "--with-notify-start-stop" ]]; then

tests/darktable_sync.bats

@@ -52,8 +52,8 @@ setup() {
     [ -f "$CONFIG_DIR/sync_pending" ]
 }
 
-@test "Lockfile wird nach Abschluss entfernt" {
+@test "Lockdir wird nach Abschluss entfernt" {
     run_with_stubs env SSH_STUB_FAIL=0 bash "$SYNC_SCRIPT"
     [ "$status" -eq 0 ]
-    [ ! -f "/tmp/darktable_sync.sh.lock" ]
+    [ ! -d "$CONFIG_DIR/sync.lock" ]
 }

tests/security.bats

@@ -0,0 +1,147 @@
+#!/usr/bin/env bats
+# Security-Tests fuer darktable-sync
+
+load helpers/setup
+
+COMMON_SCRIPT="$BATS_TEST_DIRNAME/../scripts/darktable_common.sh"
+SYNC_SCRIPT="$BATS_TEST_DIRNAME/../scripts/darktable_sync.sh"
+WRAPPER_SCRIPT="$BATS_TEST_DIRNAME/../scripts/darktable_wrapper.sh"
+
+setup() {
+    create_valid_env
+    mkdir -p "$HOME/.config/darktable"
+    touch "$HOME/.config/darktable/library.db"
+    touch "$HOME/.config/darktable/data.db"
+    mkdir -p "$HOME/Pictures"
+    export DISPLAY=:99
+}
+
+# --- K1: .env Code-Injection wird geblockt ---
+
+@test "security: .env mit Semikolon wird abgelehnt" {
+    cat > "$CONFIG_DIR/.env" <<'EOF'
+SERVER_IP=192.168.1.100
+SERVER_USER=testuser
+SERVER_SSH_PORT=22
+SERVER_DB_DIR=/remote/db
+SERVER_PHOTO_DIR=/remote/photos
+LOCAL_DARKTABLE_DB_DIR=/tmp/dt_test
+LOCAL_PHOTO_DIR=/tmp/photos_test
+DARKTABLE_BIN=darktable
+SYNC_BIN=/usr/local/bin/darktable_sync.sh
+INJECTION_MARKER=injected; touch /tmp/dt_security_test_marker
+EOF
+    run bash -c "source '$COMMON_SCRIPT'; load_config; echo done"
+    rm -f /tmp/dt_security_test_marker
+    [ "$status" -eq 1 ]
+    [[ "$output" == *"unerlaubte Zeichen"* ]]
+}
+
+@test "security: .env mit Backtick wird abgelehnt" {
+    cat > "$CONFIG_DIR/.env" <<'EOF'
+SERVER_IP=192.168.1.100
+SERVER_USER=testuser
+SERVER_SSH_PORT=22
+SERVER_DB_DIR=/remote/db
+SERVER_PHOTO_DIR=/remote/photos
+LOCAL_DARKTABLE_DB_DIR=/tmp/dt_test
+LOCAL_PHOTO_DIR=/tmp/photos_test
+DARKTABLE_BIN=darktable
+SYNC_BIN=/usr/local/bin/darktable_sync.sh
+EVIL=`touch /tmp/evil`
+EOF
+    run bash -c "source '$COMMON_SCRIPT'; load_config; echo done"
+    [ "$status" -eq 1 ]
+    [[ "$output" == *"unerlaubte Zeichen"* ]]
+}
+
+# --- K2: validate_path blockt SSH-Injection ---
+
+@test "security: SERVER_DB_DIR mit Single-Quote wird geblockt" {
+    create_valid_env
+    # Wert in Double-Quotes damit bash ihn fehlerfrei laedt, validate_path muss dann blockieren
+    printf 'SERVER_DB_DIR="/remote/db'"'"'injection"\n' >> "$CONFIG_DIR/.env"
+    run bash -c "source '$COMMON_SCRIPT'; load_config; validate_config"
+    [ "$status" -eq 1 ]
+    [[ "$output" == *"SERVER_DB_DIR"* ]]
+}
+
+@test "security: SERVER_DB_DIR mit Path-Traversal wird geblockt" {
+    create_valid_env
+    echo 'SERVER_DB_DIR=/../../../etc' >> "$CONFIG_DIR/.env"
+    run bash -c "source '$COMMON_SCRIPT'; load_config; validate_config"
+    [ "$status" -eq 1 ]
+    [[ "$output" == *"SERVER_DB_DIR"* ]]
+}
+
+# --- H1: Atomares Locking mit mkdir ---
+
+@test "security: gleichzeitiger Sync wird durch Lockdir geblockt" {
+    mkdir -p "$CONFIG_DIR/sync.lock"
+    run_with_stubs env SSH_STUB_FAIL=0 bash "$SYNC_SCRIPT"
+    [ "$status" -eq 1 ]
+    [[ "$output" == *"laeuft bereits"* ]]
+    rmdir "$CONFIG_DIR/sync.lock"
+}
+
+# --- H2: Lockdir nicht durch Symlink angreifbar ---
+
+@test "security: Lockdir ist kein Symlink-Angriffspunkt" {
+    # mkdir schlaegt bei existierendem Symlink fehl – kein Ziel wird geloescht
+    TARGET="$BATS_TMPDIR/symlink_target"
+    echo "wichtiger Inhalt" > "$TARGET"
+    ln -sf "$TARGET" "$CONFIG_DIR/sync.lock"
+
+    run_with_stubs env SSH_STUB_FAIL=0 bash "$SYNC_SCRIPT"
+    # Script muss fehlschlagen (Symlink statt echtes Verzeichnis = mkdir schlaegt fehl)
+    [ "$status" -eq 1 ]
+    # Zieldatei darf nicht geloescht worden sein
+    [ -f "$TARGET" ]
+    rm -f "$CONFIG_DIR/sync.lock" "$TARGET"
+}
+
+# --- H3: DARKTABLE_BIN muss 'darktable' sein ---
+
+@test "security: DARKTABLE_BIN mit anderem basename wird geblockt" {
+    create_valid_env
+    echo "DARKTABLE_BIN=/usr/bin/evil_binary" >> "$CONFIG_DIR/.env"
+    run bash -c "source '$COMMON_SCRIPT'; load_config; validate_config"
+    [ "$status" -eq 1 ]
+    [[ "$output" == *"DARKTABLE_BIN"* ]]
+}
+
+# --- M2: .env-Berechtigungen werden gewarnt ---
+
+@test "security: .env mit world-readable Berechtigungen loest Warnung aus" {
+    create_valid_env
+    chmod 644 "$CONFIG_DIR/.env"
+    run bash -c "source '$COMMON_SCRIPT'; load_config; echo \$SERVER_IP"
+    [ "$status" -eq 0 ]
+    [[ "$output" == *"world-readable"* ]]
+}
+
+# --- validate_config: fehlende Variablen ---
+
+@test "security: validate_config blockt leere SERVER_IP" {
+    create_valid_env
+    echo "SERVER_IP=" >> "$CONFIG_DIR/.env"
+    run bash -c "source '$COMMON_SCRIPT'; load_config; validate_config"
+    [ "$status" -eq 1 ]
+    [[ "$output" == *"SERVER_IP"* ]]
+}
+
+@test "security: validate_config blockt fehlende SERVER_DB_DIR" {
+    create_valid_env
+    sed -i '/^SERVER_DB_DIR/d' "$CONFIG_DIR/.env"
+    run bash -c "source '$COMMON_SCRIPT'; load_config; validate_config"
+    [ "$status" -eq 1 ]
+    [[ "$output" == *"SERVER_DB_DIR"* ]]
+}
+
+# --- Lockdir Cleanup ---
+
+@test "security: Lockdir wird bei normalem Exit entfernt" {
+    run_with_stubs env SSH_STUB_FAIL=0 bash "$SYNC_SCRIPT"
+    [ "$status" -eq 0 ]
+    [ ! -d "$CONFIG_DIR/sync.lock" ]
+}

uninstall.sh

@@ -19,12 +19,12 @@ systemctl --user disable --now darktable-sync.timer 2>/dev/null || true
 systemctl --user disable --now darktable_sync.timer 2>/dev/null || true
 systemctl --user daemon-reload
 
-### Lockfile entfernen
+### Lockdir entfernen (atomares Lock)
 
-LOCKFILE="/tmp/darktable_sync.sh.lock"
+LOCKDIR="$CONFIG_DIR/sync.lock"
-if [ -f "$LOCKFILE" ]; then
+if [ -d "$LOCKDIR" ]; then
-    echo "Lockfile entfernen: $LOCKFILE"
+    echo "Lockdir entfernen: $LOCKDIR"
-    rm -f "$LOCKFILE"
+    rmdir "$LOCKDIR" 2>/dev/null || true
 fi
 
 ### Aktiven Marker auf Server entfernen (best-effort)