Pitfalls after rebuild and switching PHP from DSO to suPHP

[cpanelkenneth]

I appreciate your proof of concept, but my understanding of the script is that the context in which the line

/bin/chown -R ${CPUSER}:${CPUSER} -- ${CPHOME}

appears, means that $CPUSER and $CPHOME can't ever be "root" or "/root" because it is encased within a while loop that starts:

/bin/ls -- /var/cpanel/users | /bin/grep -v "\`\|\.\|cpanel\|root\|mysql\|nobody" | while read CPUSER; do

So, I can't see how it is possible for this script to expose this exploit. However, it's late on Friday and I have been wrong before... often...

Because

/bin/chown -R ${CPUSER}:${CPUSER} -- ${CPHOME}

is the same as

chown -R homer:homer /home/homer/public_html/*

in my proof of concept. Notice that the command is not relying upon sending root:root or operating on /root, the flaw is in blindly chowning files as root.

All that this line:

/bin/ls -- /var/cpanel/users | /bin/grep -v "\`\|\.\|cpanel\|root\|mysql\|nobody" | while read CPUSER; do

does is ensure the chown step doesn't do this:

chown -R root:root /home/homer/public_html/*

which would be harmful in another manner.

So, let's do a step-by-step commentary on a few lines from this script:

/bin/ls -- /var/cpanel/users | /bin/grep -v "\`\|\.\|system\|cpanel\|root\|mysql\|nobody" | while read CPUSER; do

Get a list of cPanel users, filtering out root, system, cpanel, mysql, nobody, ` and . Iterate over this list.

   CPHOME="$(/bin/grep "^${CPUSER}:" /etc/passwd | cut -d':' -f6)/public_html"

Grab the location of the home directory for the user. Set the bash variable to /home/user/public_html

   dialog --keep-window --title "Checking ${CPHOME}" --infobox "\n\n Updating ${CPUSER}'s account ..." 10 50
   sleep ${DELAY}   # Slow things down so you can see dialog message
   echo "Checking ${CPHOME} ... "

Provide some feedback

   if [ -d "${CPHOME}" ]; then

Ensure /home/user/public_html is directory

     echo "Setting global ownership in ${CPHOME} to ${CPUSER} ..."

Feedback

     /bin/chown -R ${CPUSER}:${CPUSER} -- ${CPHOME}/*

Blindly change ownership of everything in /home/user/public_html to user:user. This is the problem line illustrated in my proof of concept.

If you doubt it, you are free to perform your own analysis and tests. I'm simply providing a warning and information on the danger of executing this script without performing due diligence.

[santrix]

Ah! Duh!

Something learnt there...

by creating a hard link to the file /etc/shadow owned by root, you have created a virtual php file with ownership set to root...

Now, running the command

/bin/chown -R ${CPUSER}:${CPUSER} -- ${CPHOME}

AS ROOT... this means you are forcing the virtual php file, ergo the original file to become owned by the mere mortal user... Arrgghh!! now I see.

So, the safest way to run this script would actually be:

/usr/bin/sudo -u homer chown -R ${CPUSER}:${CPUSER} -- ${CPHOME}

Although not knowing sudo usage too well, I expect that may need refining, but you get the gist... i.e. if the command was run under the context of the user account being processed, then it wouldn't be able to overwrite root's ownership of the original /etc/shadow file?

[cpanelkenneth]

Ah! Duh!

Something learnt there...

by creating a hard link to the file /etc/shadow owned by root, you have created a virtual php file with ownership set to root...

Now, running the command

/bin/chown -R ${CPUSER}:${CPUSER} -- ${CPHOME}

AS ROOT... this means you are forcing the virtual php file, ergo the original file to become owned by the mere mortal user... Arrgghh!! now I see.

So, the safest way to run this script would actually be:

/usr/bin/sudo -u homer chown -R ${CPUSER}:${CPUSER} -- ${CPHOME}

Although not knowing sudo usage too well, I expect that may need refining, but you get the gist... i.e. if the command was run under the context of the user account being processed, then it wouldn't be able to overwrite root's ownership of the original /etc/shadow file?

This:

/usr/bin/sudo -u homer chown -R ${CPUSER}:${CPUSER} -- ${CPHOME}

won't accomplish one of your goals which is to change homer:nobody or nobody:nobody to homer:homer. Only the root user can change ownership of a file.

One solution I'm aware of to prevent this ( and likely not the only solution ) is to:

1. Lock the directory so the user cannot access it
2. For each file in the directory:
a. verify the file is not a hard link to another file
b. if the file is not a hard link, change ownership
c. if the file is a hard link, skip processing this file
3. After all files in the directory are processed, unlock the directory

By locking the directory, I mean an action:

chown root:root directory_name
chmod 0700

And then reversing the above for step 3.

The reason for locking the directory is to prevent a race condition between verifying a file is not a hard link to another file and the ownership change. Since the check and the change are two separate actions a malicious user could change the file between the two actions.

Some information on this ( hopefully not sleep inducing ) is found here: Avoid Race Conditions

[santrix]

Yeah, I figured this out just after I wrote my reply and tested it in a shell...

I went and ran the script on a new server (eeek!)... I've just written another script to check for files belonging to users account that exist outside of their home directories... only a few (trusted) users on this box, and luckily nothing suspicious... lesson well and truly learnt!

Now... it's friday... it's 19:20hrs local, and I must be one sad git for being sat here and not consuming beer... Have a good weekend!

[Spiral]

Please forgive my dumbness, but why, under suPHP, would you want

executable for the owner
executable or writable by the group
any permissions for nobody directly

I'm sure there are some devious reasons, but humour me

Under normal circumstances you don't need executable anything but there are some special exceptions and unusual configurations that you must account for as well when writing a script to be used by everyone on many different types of servers under different configurations and more specifically with different software applications including a couple specific ones I know to be a bit screwy in their design.

Under normal circumstances barring any strangeness like that, the following is the permissions you would want normally (SuPHP/FCGI):

755 All Folders
600 PHP Scripts (most all PHP scripts)
400 PHP Scripts (those that don't like being writable *RARE*)
755 CGI Scripts (*.pl, *.pm, *py, *.e, *.cgi)
644 All Other Files (Images, Templates, HTML, Text, Etc)

Aren't htaccess files more vulnerable now that apache is running in the the same context as the hosting account (and therefore has the priveledge to overwrite the htaccess files?)

No! And for your information Apache is **NOT** running as the users but rather instead is still running as "nobody" same as always!

Your scripts however are being executed under your username which is actually better for you because you won't run into that situation like you just mentioned where you had difficulty editing the .htaccess because the file was owned by user "nobody".

If you want to take things a step above and beyond, you can set the permission of owner on the .htaccess to "4" which disables write access to yourself and put in a deny block for .htaccess inside the .htaccess which is still read just fine by Apache but won't allow visitor direct access

[Spiral]

Please be aware that the script Spiral provides ( Pitfalls after rebuild and switching PHP from DSO to suPHP ) allows user's to take over arbitrary files on the same file system as /home. This is due to this step in his script:

You owe me a very serious apology and you are also wrong on all counts ...

1. The code you keep quoting is not my script and you keep
wrongly attributing that to my name to that and I would
sincerely appreciate you stop doing that!

2. You are also mixing 2 different discussions and 2 different scripts
and that provides for a lot of confusion!

3. You are also wrong on the "previously warned" comment as the
one and only time this subject was ever brought up was totally out of
context and flaming against just a quick 30 second code sample
answering a user's specific question HOW to do something only
as an answer to their question and a framework that they could
work from and was not by any means a complete functioning
script or anything final and usable by any means but apparently
you missed that entirely as well.

4. The utility that I have available for converting dso to suphp is
not the script that you are discussing in this thread and the lines you
quoted in your discussions bare absolutely no resemblance
whatsoever to my script either!

(Which incidentally you aren't even using the correct language!)

It is perfectly fine to be debating the code lines of whatever script it is that santrix is using but this isn't my script so I would appreciate not getting that confused in your posts and discussions.

[cpanelkenneth]

You owe me a very serious apology and you are also wrong on all counts ...

1. The code you keep quoting is not my script and you keep
wrongly attributing that to my name to that and I would
sincerely appreciate you stop doing that!

2. You are also mixing 2 different discussions and 2 different scripts
and that provides for a lot of confusion!

3. You are also wrong on the "previously warned" comment as the
one and only time this subject was ever brought up was totally out of
context and flaming against just a quick 30 second code sample
answering a user's specific question HOW to do something only
as an answer to their question and a framework that they could
work from and was not by any means a complete functioning
script or anything final and usable by any means but apparently
you missed that entirely as well.

4. The utility that I have available for converting dso to suphp is
not the script that you are discussing in this thread and the lines you
quoted in your discussions bare absolutely no resemblance
whatsoever to my script either!

(Which incidentally you aren't even using the correct language!)

It is perfectly fine to be debating the code lines of whatever script it is that santrix is using but this isn't my script so I would appreciate not getting that confused in your posts and discussions.

Thank you for removing the script, that will prevent unintended security problems for people. Please desist from misrepresenting what happened.


This thread no longer serves a purpose, other than one of caution.