security: fix critical security issues
- 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>
This commit is contained in:
16
CHANGELOG.md
16
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)
|
||||
|
||||
67
samba.sh
67
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
|
||||
|
||||
Reference in New Issue
Block a user