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 (jeff@commentedcode.org)
#  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="webmaster@example.com"
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
\$\endgroup\$
  • \$\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\$ – Peter Cordes Sep 22 '15 at 23:09
  • \$\begingroup\$ piping passwords seem to work well. \$\endgroup\$ – jecxjo Sep 23 '15 at 0:46
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.

\$\endgroup\$
  • \$\begingroup\$ @jecxjo: updated my answer. \$\endgroup\$ – Peter Cordes Sep 23 '15 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\$ – jecxjo Sep 23 '15 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\$ – Peter Cordes Sep 23 '15 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\$ – Peter Cordes Sep 23 '15 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\$ – jecxjo Sep 23 '15 at 21:26
3
\$\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

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.