From 8c17a559b909930295beccdd0fef737868e02ac6 Mon Sep 17 00:00:00 2001
From: Struchkov Mark <mstruchkov@nota.tech>
Date: Wed, 7 Jan 2026 22:21:10 +0300
Subject: [PATCH] security: fix critical security issues
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Use heredoc instead of echo for password input (not visible in ps/proc)
- Add username validation (alphanumeric, underscore, hyphen only)
- Add share name validation (alphanumeric, space, underscore, hyphen)
- Add path validation (must be absolute)
- Replace 'which' with POSIX-compliant 'command -v'
- Add logging for custom command execution
- Improve error handling with proper quoting

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 CHANGELOG.md | 16 +++++++++++++
 samba.sh     | 67 ++++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 71 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 260375e..5fd7d57 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,22 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   - `max xmit = 65535` — maximum packet size for better throughput
   - `write cache size = 1048576` — 1MB write cache for improved write performance
 
+### Security
+
+- **Secure password handling in user() function** (samba.sh)
+  - Replaced `echo` with heredoc for password input to prevent exposure in process list
+  - Password is no longer visible via `ps` or `/proc`
+
+- **Input validation for users and shares** (samba.sh)
+  - Added username validation (alphanumeric, underscore, hyphen only)
+  - Added share name validation (alphanumeric, space, underscore, hyphen only)
+  - Added path validation (must be absolute path)
+
+- **Improved command execution safety** (samba.sh)
+  - Replaced `which` with POSIX-compliant `command -v`
+  - Added logging for custom command execution
+  - Improved error messages
+
 ### Fixed
 
 - **Fixed chown syntax error in perms function** (samba.sh:122)
diff --git a/samba.sh b/samba.sh
index ef8f023..3dc1bd3 100755
--- a/samba.sh
+++ b/samba.sh
@@ -152,6 +152,18 @@ share() {
           guest="${5:-yes}" users="${6:-""}" admins="${7:-""}" \
           writelist="${8:-""}" comment="${9:-""}"
 
+    # Validate share name (alphanumeric, space, underscore, hyphen only)
+    if [[ ! "$share" =~ ^[a-zA-Z0-9\ _-]+$ ]]; then
+        echo "ERROR: Invalid share name format: $share" >&2
+        return 1
+    fi
+
+    # Validate path (must be absolute)
+    if [[ ! "$path" =~ ^/ ]]; then
+        echo "ERROR: Share path must be absolute: $path" >&2
+        return 1
+    fi
+
     sed -i "/\\[$share\\]/,/^\$/d" "$SMB_CONF"
     echo "[$share]" >> "$SMB_CONF"
     echo "   path = $path" >> "$SMB_CONF"
@@ -192,13 +204,39 @@ smb() {
 #   group) for user
 #   gid) for group
 # Return: user added to container
-user() { local name="$1" passwd="$2" id="${3:-""}" group="${4:-""}" \
-                gid="${5:-""}"
-    [[ "$group" ]] && { grep -q "^$group:" /etc/group ||
-                addgroup ${gid:+--gid $gid }"$group"; }
-    grep -q "^$name:" /etc/passwd ||
-        adduser -D -H ${group:+-G $group} ${id:+-u $id} "$name"
-    echo -e "$passwd\n$passwd" | smbpasswd -s -a "$name"
+user() {
+    local name="$1" passwd="$2" id="${3:-""}" group="${4:-""}" gid="${5:-""}"
+
+    # Validate username (alphanumeric, underscore, hyphen only)
+    if [[ ! "$name" =~ ^[a-zA-Z0-9_-]+$ ]]; then
+        echo "ERROR: Invalid username format: $name" >&2
+        return 1
+    fi
+
+    # Create group if specified and doesn't exist
+    if [[ -n "$group" ]]; then
+        if ! grep -q "^$group:" /etc/group; then
+            if [[ -n "$gid" ]]; then
+                addgroup --gid "$gid" "$group"
+            else
+                addgroup "$group"
+            fi
+        fi
+    fi
+
+    # Create user if doesn't exist
+    if ! grep -q "^$name:" /etc/passwd; then
+        local adduser_args="-D -H"
+        [[ -n "$group" ]] && adduser_args="$adduser_args -G $group"
+        [[ -n "$id" ]] && adduser_args="$adduser_args -u $id"
+        adduser $adduser_args "$name"
+    fi
+
+    # Set password securely using heredoc (not visible in process list)
+    smbpasswd -s -a "$name" <<EOF
+$passwd
+$passwd
+EOF
 }
 
 ### workgroup: set the workgroup
@@ -342,11 +380,16 @@ done < <(env | awk '/^USER[0-9=_]/ {sub (/^[^=]*=/, "", $0); print}')
 [[ "${INCLUDE:-""}" ]] && include "$INCLUDE"
 [[ "${PERMISSIONS:-""}" ]] && perms &
 
-if [[ $# -ge 1 && -x $(which $1 2>&-) ]]; then
-    exec "$@"
-elif [[ $# -ge 1 ]]; then
-    echo "ERROR: command not found: $1"
-    exit 13
+if [[ $# -ge 1 ]]; then
+    # Validate command exists and is executable
+    cmd_path=$(command -v "$1" 2>/dev/null)
+    if [[ -n "$cmd_path" && -x "$cmd_path" ]]; then
+        echo "INFO: Executing custom command: $1" >&2
+        exec "$@"
+    else
+        echo "ERROR: command not found or not executable: $1" >&2
+        exit 13
+    fi
 elif ps -ef | grep -E -v grep | grep -q smbd; then
     echo "Service already running, please restart container to apply changes"
 else