4
\$\begingroup\$

All the services I run on my server are based on Unix accounts. Since most web services have their own users and perform all the account management separate from the actual system accounts, I created a CGI script that handles:

  1. Changing passwords (requires old password)
  2. Assigning contact info (requires password)
  3. Request password reset (no passwords sent in email)

I've tried to use only system commands and no external scripts (aside from the one to get POST variables). The application is not run setuid, but permissions are required for sudo to run chpasswd.

I'm looking for any issues with sanitizing form data, how I'm using expect to input to system commands, etc. I know I can clean up the code a bit and refactor all the duplicate code. Basically I got the thing working and now and am looking for how to make it better before I start cleaning it up.

Code posted on Github

#!/bin/bash - 
#===============================================================================
# Copyright (c) 2015 Jeff Parent
# All rights reserved.
#
# Redistribution and use in source and binary forms, with or without modification,
# are permitted provided that the following conditions are met:
#
# * Redistributions of source code must retain the above copyright notice, this
# list of conditions and the following disclaimer.
# * Redistributions in binary form must reproduce the above copyright notice,
# this list of conditions and the following disclaimer in the documentation
# and/or other materials provided with the distribution.
# * Neither the name of the passwd.sh authors nor the names of its contributors
# may be used to endorse or promote products derived from this software without
# specific prior written permission.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#
# FILE: passwd.sh
#
# USAGE: ./passwd.sh 
#
# DESCRIPTION: cgi script to modify unix passwords
#
# OPTIONS: ---
# REQUIREMENTS: sudo access to chpasswd
# BUGS: ---
# NOTES: ---
# AUTHOR: jecxjo ([email protected])
# ORGANIZATION:
# CREATED: 09/20/15 13:28
# REVISION: 0.0.4
#
# CHANGELOG: 0.0.4 - Security holes and cleanup
# 0.0.3 - Moved bash_cgi code to file
# 0.0.2 - Clean up
# 0.0.1 - Initial version
#
#===============================================================================
# Global Vars
URL="https://example.com/cgi-bin/passwd.sh"
TITLE="Change Password"
EMAIL_FROM_NAME="Webmaster"
EMAIL_FROM_ADDRESS="[email protected]"
USER_DB="/var/lib/passwd.sh/users.db"
RESET_DB="/var/lib/passwd.sh/reset.db"
RND_CMD=$(/usr/bin/dd if=/dev/random bs=1 count=32 | \
 /usr/bin/base64 | \
 /usr/bin/sed 's|+||g' | \
 /usr/bin/sed 's|/||g' | \
 /usr/bin/sed 's|=||g' | \
 /usr/bin/sed 's| ||g')
BLACKLIST=(root http nobody)
#################
# Confirm Reset #
#################
# Apply new password and output HTML status
# 1->user, 2->pass
function ResetPass () {
 local usr=$(IsSaneUser "1ドル") pass="2ドル"
 # write new user:pass to system
 echo "${usr}:${pass}" | /usr/bin/sudo /usr/bin/chpasswd
 # Check if password change was successful
 if [ $? -eq 0 ]; then
 echo "<b>Success:</b> Password changed successfully<br />"
 # Remove all instances of reset keys
 /usr/bin/umask 026
 local tmp=$(mktemp /tmp/reset.XXXXXX)
 sed "/:${usr}$/d" "${RESET_DB}" > "${tmp}"
 mv "${tmp}" "${RESET_DB}"
 /usr/bin/chown http:http "${RESET_DB}"
 else
 echo "<b>Error:</b> Failed setting password<br />"
 fi
}
# Check if Key:User DB and return HTML Form to reset
# 1->user, 2->key
function ConfirmReset () {
 local usr=$(IsSaneUser "1ドル") key="2ドル"
 /usr/bin/grep -q "^${key}:${usr}" "${RESET_DB}"
 # Check if reset code is valid
 if [ $? -eq 0 ]; then
 # Create form to enter new password
 /usr/bin/cat <<EOF
<form action="${URL}" method="POST">
 <fieldset>
 <legend>Reset Password</legend>
 <input type="hidden" name="cmd" id="cmd" value="resetpass" />
 <input type="hidden" name="key" id="key" value="${key}" />
 <input type="hidden" name="user" id="user" value="${usr}" />
 <p><label class="field" for="pass">Password:</label><input type="password" name="pass" id="pass" class="textbox-300" /></p>
 <p><label class="field" for="passcfm">Confirm:</label><input type="password" name="passcfm" id="passcfm" class="textbox-300" /></p>
 <input type="submit" value="Submit" />
 </fieldset>
</form>
EOF
 else
 echo "<b>Error:</b> Reset code is not valid<br />"
 fi
}
# Check if all form data is valid for new password on reset
# as generated by ConfirmReset
# 1->user, 2->key, 3->pass, 4->cfm
function ApplyNewPass () {
 local usr=$(IsSaneUser "1ドル") key="2ドル" pass="3ドル" cfm="4ドル"
 if [ -z "${usr}" ]; then
 echo "<b>Error:</b> No User entered<br />"
 elif [ -z "${key}" ]; then
 echo "<b>Error:</b> No Key<br />"
 elif [ -z "${pass}" ]; then
 echo "<b>Error:</b> No New Password<br />"
 ConfirmReset "${usr}" "${key}"
 elif [ -z "${cfm}" ]; then
 echo "<b>Error:</b> No New Password<br />"
 ConfirmReset "${usr}" "${key}"
 else
 grep -q "^${usr}:" /etc/passwd
 if [ $? -eq 1 ]; then
 echo "<b>Error:</b> User does not exist<br />"
 elif [ "${pass}" != "${cfm}" ]; then
 echo "<b>Error:</b> New Passwords don't match<br />"
 ConfirmReset "${usr}" "${key}"
 else
 ResetPass "${usr}" "${pass}"
 fi
 fi
}
##################
# Password Reset #
##################
# Find Email Address from Contact Info
# 1->user
function GetAddress () {
 local usr=$(IsSaneUser "1ドル")
 /usr/bin/awk -v search="^${usr}:" '0ドル ~ search{split(0,ドルa,":"); print a[2];}' "${USER_DB}"
}
# Create form to request Reset Email
# 1->user
function UserResetForm () {
 local user=$(IsSaneUser "1ドル")
 /usr/bin/cat <<EOF
<form action="${URL}" method="POST">
 <fieldset>
 <legend>Reset Password</legend>
 <input type="hidden" name="cmd" id="cmd" value="setreset" />
 <p><label class="field" for="user">User:</label><input type="text" name="user" id="user" class="textbox-300" value="${user}" /></p>
 <input type="submit" value="Submit" />
 </fieldset>
</form>
EOF
}
# Create Email, send it and then generate HTML status
# 1->user
function ApplyReset () {
 local usr=$(IsSaneUser "1ドル")
 local key="${RND_CMD}"
 if [ -z "${usr}" ]; then
 echo "<b>Error:</b> No User entered<br />"
 UserResetForm ""
 else
 /usr/bin/grep -q "^${usr}:" /etc/passwd
 if [ $? -eq 1 ]; then
 echo "<b>Error:</b> User does not exist<br />"
 UserResetForm ""
 else
 /usr/bin/grep -q "^${usr}:" "${USER_DB}"
 if [ $? -eq 1 ]; then
 echo "<b>Error:</b> User has no contact info<br />"
 UserResetForm ""
 else
 # Create Email message
 local subject="Password Reset"
 local link="${URL}?cmd=cfmreset&user=${usr}&key=${key}"
 local address=$(GetAddress "${usr}")
 local message=$(cat <<EOF
A request was made to reset the password for ${usr}. If this was in error
please ignore this message. Otherwise follow the link to reset your account
password:
${link}
Thank you
EOF)
 local mail="subject:${subject}\nfrom:${EMAIL_FROM_ADDRESS}\n\n${message}"
 echo -e "${mail}" | /usr/bin/sendmail -F "${EMAIL_FROM_NAME}" -f "${EMAIL_FROM_ADDRESS}" "${address}"
 if [ $? -eq 0 ]; then
 echo "<b>Success:</b> Email sent<br />"
 # Write key to database
 echo "${key}:${usr}" >> "${RESET_DB}"
 else
 echo "<b>Error:</b> Failed sending email<br />"
 fi
 fi
 fi
 fi
}
################
# Set Password #
################
# Create form to change password
# 1->user 2->old_pass
function UserPassForm () {
 local user=$(IsSaneUser "1ドル")
 local old_pass=2ドル
 /usr/bin/cat <<EOF
<form action="${URL}" method="POST">
 <fieldset>
 <legend>Change Password</legend>
 <input type="hidden" name="cmd" id="cmd" value="setpass" />
 <p><label class="field" for="user">User:</label> <input type="text" name="user" id="user" value="${user}" /></p>
 <p><label class="field" for="oldpass">Old Password:</label><input type="password" name="oldpass" id="oldpass" value="${old_pass}" /></p>
 <p><label class="field" for="pass">New Password:</label> <input type="password" name="pass" id="pass" /></p>
 <p><label class="field" for="passcfm">Confirm Password:</label> <input type="password" name="passcfm" id="passcfm" /></p>
 <input type="submit" value="Submit" />
 </fieldset>
</form>
EOF
}
# Apply new password to user and generate HTML status
# 1->user, 2->old pass, 3->new pass
function SetPass () {
 local user=$(IsSaneUser "1ドル") pass=2ドル new=3ドル
 local out=$(echo -e "${pass}\n${pass}\n${new}\n${new}" | /usr/bin/su -c 'if /usr/bin/passwd; then echo "SUCCESS"; fi ' "${user}")
 echo "${out}" | /usr/bin/grep -q "SUCCESS"
 if [ $? -eq 0 ]; then
 echo "<b>Success:</b> Password Changed<br />"
 else
 echo "<b>Error:</b> Failed changing password[${out}]<br />"
 fi
}
# Validate form data generated by UserPassForm
# 1->user, 2->old, 3->newa, 4->newb
function ApplyPass () {
 local usr=$(IsSaneUser "1ドル")
 local old="2ドル"
 local newa="3ドル"
 local newb="4ドル"
 if [ -z "${usr}" ]; then
 echo "<b>Error:</b> Invalid User<br />"
 UserPassForm "" ""
 elif [ -z "${old}" ]; then
 echo "<b>Error:</b> No Old Password<br />"
 UserPassForm "${usr}" ""
 elif [ -z "${newa}" ]; then
 echo "<b>Error:</b> No New Password<br />"
 UserPassForm "${usr}" "${old}"
 elif [ -z "${newb}" ]; then
 echo "<b>Error:</b> No New Password<br />"
 UserPassForm "${usr}" "${old}"
 else
 /usr/bin/grep -q "^${usr}:" /etc/passwd
 if [ $? -eq 1 ]; then
 echo "<b>Error:</b> User does not exist<br />"
 UserPassForm "" ""
 elif [ "${newa}" != "${newb}" ]; then
 echo "<b>Error:</b> New Passwords don't match<br />"
 UserPassForm "${usr}" "${old}"
 else
 SetPass "${usr}" "${old}" "${newa}"
 fi
 fi
}
################
# Contact Info #
################
# Create form to update Contact Info
# 1->user, 2->email
function UserContactForm () {
 local user=$(IsSaneUser "1ドル") email=$(IsSaneEmail "2ドル")
 /usr/bin/cat <<EOF
<form action="${URL}" method="POST">
 <fieldset>
 <legend>Change Contact Info</legend>
 <input type="hidden" name="cmd" id="cmd" value="setcontact" />
 <p><label class="field" for="user">User:</label><input type="text" name="user" id="user" value="${user}" /></p>
 <p><label class="field" for="user">Password:</label><input type="password" name="pass" id="pass" /></p>
 <p><label class="field" for="user">Email:</label><input type="email" name="email" id="email" value="${email}" /></p>
 <input type="submit" value="Submit" />
 </fieldset>
</form>
EOF
}
# Apply new contact info and generate HTML status
# 1->user, 2->password, 3->email
function SetContact () {
 local usr=$(IsSaneUser "1ドル") pass=2ドル email=$(IsSaneEmail "3ドル")
 local f="/tmp/${usr}"
 local str="${usr}:${email}"
 # Touch file as user, requires correct password
 local out=$(echo -e "${pass}\n" | /usr/bin/su -c "/usr/bin/touch \"${f}\"" - "${usr}")
 # if su worked, user/pass was valid
 if [ -e "${f}" ]; then
 # Remove old contact info and add new
 /usr/bin/umask 026
 TMP=$(/usr/bin/mktemp /tmp/contact.XXXXXX)
 /usr/bin/sed "/^${usr}:/d" "${USER_DB}" > "${TMP}"
 echo "${usr}:${email}" >> "${TMP}"
 mv "${TMP}" "${USER_DB}"
 /usr/bin/chown http:http "${USER_DB}"
 echo "<b>Success:</b> Contact Info Updated<br />"
 # cleanup touched file
 local out=$(echo -e "${pass}\n" | /usr/bin/su -c "/usr/bin/rm \"${f}\"" - "${usr}")
 else
 echo "<b>Error:</b> Failed to update DB, No file[${f}]<br />"
 fi
}
# Validate form data generated by UserContactForm
# 1->user, 2->pass, 3->email
function ApplyContact () {
local usr=$(IsSaneUser "1ドル") pass="2ドル" email=$(IsSaneEmail "3ドル")
 if [ -z "${usr}" ]; then
 echo "<b>Error:</b> Invalid Username<br />"
 UserContactForm "" "${email}"
 elif [ -z "${pass}" ]; then
 echo "<b>Error:</b> Invalid Password<br />"
 UserContactForm "${usr}" "${email}"
 elif [ -z "${email}" ]; then
 echo "<b>Error:</b> Invalide Email<br />"
 UserContactForm "${usr}" ""
 else
 grep -q "^${usr}:" /etc/passwd
 if [ $? -eq 1 ]; then
 echo "<b>Error:</b> User does not exist<br />"
 UserContactForm "" "${email}"
 else
 SetContact "${usr}" "${pass}" "${email}"
 fi
 fi
}
####################
# Main Application #
####################
# Switch on URL argument "cmd" to generate correct page
# 1ドル->cmd
function Body () {
 echo "<body>"
 cgi_getvars BOTH cmd
 case "${cmd}" in
 resetpass)
 cgi_getvars BOTH user
 cgi_getvars BOTH key
 cgi_getvars BOTH pass
 cgi_getvars BOTH passcfm
 ApplyNewPass "${user}" "${key}" "${pass}" "${passcfm}"
 ;;
 cfmreset)
 cgi_getvars BOTH user
 cgi_getvars BOTH key
 ConfirmReset "${user}" "${key}"
 ;;
 setreset)
 cgi_getvars BOTH user
 ApplyReset "${user}"
 ;;
 setpass)
 cgi_getvars BOTH user
 cgi_getvars BOTH oldpass
 cgi_getvars BOTH pass
 cgi_getvars BOTH passcfm
 ApplyPass "${user}" "${oldpass}" "${pass}" "${passcfm}"
 ;;
 setcontact)
 cgi_getvars BOTH user
 cgi_getvars BOTH pass
 cgi_getvars BOTH email
 ApplyContact "${user}" "${pass}" "${email}"
 ;;
 resetform)
 cgi_getvars BOTH user
 UserResetForm "${user}"
 ;;
 contactform)
 cgi_getvars BOTH user
 cgi_getvars BOTH email
 UserContactForm "${user}" "${email}"
 ;;
 passform)
 cgi_getvars BOTH user
 cgi_getvars BOTH oldpass
 UserPassForm "${user}" "${oldpass}"
 ;;
 *)
 cgi_getvars BOTH user
 cgi_getvars BOTH oldpass
 UserPassForm "${user}" "${oldpass}"
 ;;
 esac
 echo "<br />"
 echo "<a href=\"/cgi-bin/passwd.sh?cmd=passform\">Password</a>"
 echo "<a href=\"/cgi-bin/passwd.sh?cmd=contactform\">Contact</a>"
 echo "<a href=\"/cgi-bin/passwd.sh?cmd=resetform\">Reset Password</a>"
 echo "</body>"
}
# START bash_cgi
# Created by Philippe Kehl
# http://oinkzwurgl.org/bash_cgi
# (internal) routine to store POST data
function cgi_get_POST_vars()
{
 # check content type
 # FIXME: not sure if we could handle uploads with this..
 [ "${CONTENT_TYPE}" != "application/x-www-form-urlencoded" ] && \
 echo "bash.cgi warning: you should probably use MIME type "\
 "application/x-www-form-urlencoded!" 1>&2
 # save POST variables (only first time this is called)
 [ -z "$QUERY_STRING_POST" \
 -a "$REQUEST_METHOD" = "POST" -a ! -z "$CONTENT_LENGTH" ] && \
 read -n $CONTENT_LENGTH QUERY_STRING_POST
 # prevent shell execution
 local t
 t=${QUERY_STRING_POST//%60//} # %60 = `
 t=${t//\`//}
 t=${t//\$(//}
 t=${t//%24%28//} # %24 = ,ドル %28 = (
 QUERY_STRING_POST=${t}
 return
}
# (internal) routine to decode urlencoded strings
function cgi_decodevar()
{
 [ $# -ne 1 ] && return
 local v t h
 # replace all + with whitespace and append %%
 t="${1//+/ }%%"
 while [ ${#t} -gt 0 -a "${t}" != "%" ]; do
 v="${v}${t%%\%*}" # digest up to the first %
 t="${t#*%}" # remove digested part
 # decode if there is anything to decode and if not at end of string
 if [ ${#t} -gt 0 -a "${t}" != "%" ]; then
 h=${t:0:2} # save first two chars
 t="${t:2}" # remove these
 v="${v}"`echo -e \\\\x${h}` # convert hex to special char
 fi
 done
 # return decoded string
 echo "${v}"
 return
}
# routine to get variables from http requests
# usage: cgi_getvars method varname1 [.. varnameN]
# method is either GET or POST or BOTH
# the magic varible name ALL gets everything
function cgi_getvars()
{
 [ $# -lt 2 ] && return
 local q p k v s
 # prevent shell execution
 t=${QUERY_STRING//%60//} # %60 = `
 t=${t//\`//}
 t=${t//\$(//}
 t=${t//%24%28//} # %24 = ,ドル %28 = (
 QUERY_STRING=${t}
 # get query
 case 1ドル in
 GET)
 [ ! -z "${QUERY_STRING}" ] && q="${QUERY_STRING}&"
 ;;
 POST)
 cgi_get_POST_vars
 [ ! -z "${QUERY_STRING_POST}" ] && q="${QUERY_STRING_POST}&"
 ;;
 BOTH)
 [ ! -z "${QUERY_STRING}" ] && q="${QUERY_STRING}&"
 cgi_get_POST_vars
 [ ! -z "${QUERY_STRING_POST}" ] && q="${q}${QUERY_STRING_POST}&"
 ;;
 esac
 shift
 s=" $* "
 # parse the query data
 while [ ! -z "$q" ]; do
 p="${q%%&*}" # get first part of query string
 k="${p%%=*}" # get the key (variable name) from it
 v="${p#*=}" # get the value from it
 q="${q#$p&*}" # strip first part from query string
 # decode and evaluate var if requested
 [ "1ドル" = "ALL" -o "${s/ $k /}" != "$s" ] && \
 eval "$k=\"`cgi_decodevar \"$v\"`\""
 done
 return
}
#cgi_getvars BOTH ALL
# END of bash_cgi
################
# Sanitization #
################
# Checks if username is a sane username
# 1->user
function IsSaneUser () {
 local user=$(echo "1ドル" | /usr/bin/grep "^[0-9A-Za-z-]\+$")
 if [ ! -z "${user}" ]; then
 local count = 0
 while [ "x${BLACKLIST[count]}" != "x" ]
 do
 if [ "${user}" == "${BLACKLIST[count]}" ]; then
 return
 fi
 count=$(( ${count} + 1 ))
 done
 fi
 echo "${user}"
}
# Checks if email is sane
# 1->email
function IsSaneEmail () {
 echo "1ドル" | /usr/bin/grep -E -o "\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,6}\b"
}
#################
# CLI Functions #
#################
function PrintUsage () {
 /usr/bin/cat <<EOF
usage: passwd.sh [OPTION]
 -h | --help Print this output
 -c | --cron Trigger cleanup of expired entries
This application is a CGI script that generates a Unix account password
manager. The script should be executed as a non-root user and the user
should be given sudo access to /usr/bin/chpasswd.
To allow password reset requests to expire, this script should be run as
a cron job with the -c flag.
EOF
}
function CronMode () {
 echo "TODO: Create Cron Mode"
}
###################
# HTML Generation #
###################
function Header() {
 /usr/bin/cat <<EOF
<head>
 <title>${TITLE}</title>
 <style>
 fieldset {
 width: 500px;
 }
 legend {
 font-size: 20px;
 }
 label.field {
 text-align: right;
 width: 200px;
 float: left;
 font-weight: bold;
 }
 label.textbox-300 {
 width: 300px;
 float: left;
 }
 fieldset p {
 clear: bloth;
 padding: 5px;
 }
 </style>
</head>
EOF
}
# Cron mode, trigger cleanup of reset requests
CRON_MODE=0
# Print usage and quit
HELP_MODE=0
while [[ $# > 1 ]]
do
 local key = 1ドル
 case ${key} in
 -c|--cron)
 CRON_MODE=1
 ;;
 -h|--help)
 HELP_MODE=1
 ;;
 *)
 ;;
 esac
 shift # next arg
done
if [ ${HELP_MODE} -eq 1 ]; then
 PrintUsage
elif [ ${CRON_MODE} -eq 1 ]; then
 CronMode
else
 /usr/bin/cat <<EOF
Content-type: text/html
<!DOCTYPE html>
<html>
$(Header)
$(Body)
</html>
EOF
fi
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Sep 21, 2015 at 22:00
\$\endgroup\$
2
  • \$\begingroup\$ The password is temporarily visible in the ps output, because you're putting it on the command line of expect. If you passed the expect script with a here-document or here-string (<<< "string"), an attacker that could watch for and look at the cmdline of new processes couldn't get your passwords. Are you sure you need expect? I searched briefly, and didn't see anything definitive on good non-interactive password-changers, but probably just feeding 3 input lines into passwd or chpasswd without waiting for prompts will be ok. (obviously test it). \$\endgroup\$ Commented Sep 22, 2015 at 23:09
  • \$\begingroup\$ piping passwords seem to work well. \$\endgroup\$ Commented Sep 23, 2015 at 0:46

2 Answers 2

3
\$\begingroup\$

I haven't gone through the logic / design of the whole thing, just a few local things I saw. Another answer that looked at the overall design might find different things to say.


I guess you're running this script as root? I thought you were running it as the user that was changing their password. Since you're already root, you should just use

builtin echo -E "$user:$newpass" | chpasswd

echo -E explicitly prevents interpretation of backslash-escaped things. Since the first (and only) arg includes some fixed text (":"), you can't end up with echo -E -e printing nothing, instead of printing -e. builtin printf '%s\n' "$user:$newpass" would be equally safe.

The builtin keyword isn't needed, but sort of documents in the code the fact that those strings won't appear in the process list.


echo "${out}" | /usr/bin/grep -q "SUCCESS"

This is not a good way to do string comparisons. The [ (aka test) builtin is what you need. Or since you're using /bin/bash, not just POSIX sh, so go ahead and use the better [[ version. The wooledge bash FAQ / bash guide has some very good stuff.

if [[ "$out" != "SUCCESS" ]]; then
 stuff
else
 other stuff
fi

You don't even need that string comparison, though:

function SetPass () {
 local user=$(IsSaneUser "1ドル") pass=2ドル new=3ドル
 if out=$(echo -e "${pass}\n${pass}\n${new}\n${new}" | /usr/bin/su -c '/usr/bin/passwd' "${user}"); then
 echo "<b>Success:</b> Password Changed<br />"
 else
 echo "<b>Error:</b> Failed changing password[${out}]<br />"
 fi
}

Bash manual:

[if there are no commands (e.g. just a variable-assignment) then...] If one of the expansions contained a command substitution, the exit status of the command is the exit status of the last command substitution performed.

So it's part of the language that foo=$(echo | true) returns 0, while foo=$(echo | false) returns 1.

Since you're using bash specifically, not any old sh, you don't need printf "%s\n%s\n..." "$pass" "$pass" .... That echo is fine, and since it's a shell builtin, shouldn't show up in the process list.


Random string generation: dd from /dev/random is a good choice. You can combine the sed commands into one, though:

 /usr/bin/sed 's|+||g' | \ # \ not needed, ending with a | already means the command isn't done.
 /usr/bin/sed 's|/||g' | \
 /usr/bin/sed 's|=||g' | \
 /usr/bin/sed 's| ||g'
 sed 's|[+/= ]||g' # equivalent
 tr -d '+/= ' # equivalent

I'm not sure about this, but you should be in control of $PATH, so you shouldn't need to type out /usr/bin/... for everything. You introduced at least one bug this way: umask is a shell builtin, and has to be because it modified the shell's process environment (in the general sense, not environment variables). Luckily, there isn't a /usr/bin/umask that does something different like just printing the current umask.


cat << EOF
HTML
EOF

is good. It looks like the easiest way to get variable expansion without quote-removal. Getting double-quotes in the output of echo or printf would take messy quoting, since you'd want the whole string in double quotes. It would be a lot messier to read.

answered Sep 23, 2015 at 19:41
\$\endgroup\$
5
  • \$\begingroup\$ @jecxjo: updated my answer. \$\endgroup\$ Commented Sep 23, 2015 at 19:59
  • \$\begingroup\$ Thanks for the detailed response, Will fix all of them. A side note, the file is run as a non-root account (http) so I do require sudo access specificly for chpasswd. As for the "/usr/bin" I guess I was making sure that no env var would take the place of a given command like an alias grep="rm -rf /" type situation. \$\endgroup\$ Commented Sep 23, 2015 at 20:39
  • \$\begingroup\$ @jecxjo: Oh, I see, that's why you need the user's old passwd twice. Once for su to that user, then again for passwd. That is a better solution than giving the http user a way to become root. Still, you probably can set up your HTTP server to run these scripts as the appropriate user. Whether this is easy / convenient enough to be worth doing, only you can decide. Apache already knows how to do this to run /~user/cgi-bin/foo.cgi scripts. \$\endgroup\$ Commented Sep 23, 2015 at 21:19
  • \$\begingroup\$ and re: /usr/bin: yes, there is actually a mechanism for that. Bash looks for shell function definitions in environment variables, as made famous by the recent bash "shellshock" security bug. This lets you define a function and export it. This can't happen for aliases, though. I think you're better off making sure the environment is sanitized down to a few whitelisted entries, and with a sane $PATH, rather than trying to defend against any possible env setup. You missed one case of bare sed early on, I think I saw. Near a mktemp, IIRC. \$\endgroup\$ Commented Sep 23, 2015 at 21:24
  • \$\begingroup\$ This works in the "change password" case but for a reset (email a link to get a new pass) still needs root. I think I need a better description on the script as its sort of dual/triple purpose: Set contact info, change password, recover lost password. Sadly that last option requires root access \$\endgroup\$ Commented Sep 23, 2015 at 21:26
4
\$\begingroup\$

One big flaw with this that hasn't been mentioned so far is that the way your script is written, it requires giving the web-server uid NOPASSWD sudo access to chpasswd.

This means that if there happens to be any other exploitable code on your web server that allows a script-kiddie to run arbitrary commands, they get unrestricted use of sudo chpasswd.

Instead of allowing access to chpasswd itself, give sudo access to a wrapper script around chpasswd that very strictly checks and sanitises its arguments. Your script above would then call, e.g., sudo /usr/local/bin/mychpasswdwrapper.sh

answered Oct 25, 2015 at 3:54
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.