Ten holes that sit in every old PHP system

Anybody taking over an old PHP system gets asked about security early, usually in a form that cannot be answered: is it secure? The honest answer to that question is always no, and it helps nobody.

It becomes useful with a list. In grown PHP systems I keep finding the same ten holes, and that is no surprise: they come out of practices that were normal when the code was written.

This article goes through them, with the command that finds each one, in an order sorted by risk rather than by effort. What it is not: a substitute for a penetration test. It is what you can do yourself beforehand.

The order is by risk, not by effort

The usual mistake with a list like this is to work top to bottom or to start with the cheapest. Both leave the most dangerous hole open longest.

Sorting happens by two questions. What can an attacker achieve with it? Reading data is bad, changing data is worse, executing code is the end. And: how hard is the hole to find? What a scanner finds in five minutes will be found.

The first four points below are the ones where both answers are unfavourable. They belong in the first week, regardless of what else is planned.

The four that come first

1. Queries assembled from strings. The classic, and in code from before 2012 the rule rather than the exception. One hit is enough to read the database and usually to write to it.

# Queries with variables in them: every hit needs checking
grep -rnE '(query|exec|prepare)\s*\(\s*["'"'"'].*\$' src/ --include="*.php"

# And the places where input goes into queries unfiltered
grep -rnE '\$(_GET|_POST|_REQUEST)\[[^]]+\][^;]*(query|WHERE|SELECT)' src/

The fix is not escaping but prepared statements with parameters. The difference is not a matter of style: escaping has to be done correctly in every single place, parameters are safe by construction.

2. Passwords with outdated algorithms. md5() and sha1() on passwords are breakable in hours today, with or without a salt.

grep -rnE '(md5|sha1)\s*\(\s*\$(password|pass|pw)' src/ --include="*.php"

Switching over does not require resetting every password. On the next successful login the hash gets rewritten, and after a few months the old values are gone:

if (password_verify($input, $user->hash)) {
    // Old: md5 or sha1. Quietly raise it to the current algorithm at
    // login, without the user noticing anything.
    if (password_needs_rehash($user->hash, PASSWORD_DEFAULT)) {
        $this->users->setHash($user->id, password_hash($input, PASSWORD_DEFAULT));
    }
}

Replacing even older schemes needs a transition: check against the old hash at login, then set the new one. Anybody who has not logged in for twelve months gets a reset on their next attempt.

3. Uploaded files landing in the web directory. When an upload sits under a path the web server serves and the extension is not checked, somebody can upload a PHP file and call it. That is the hole where data access turns into code execution.

grep -rn "move_uploaded_file" src/ --include="*.php" -A 3

Checking the extension is not enough, because the file name comes from the sender. Three things together carry: the storage location sits outside the served directory, the file name is newly assigned rather than adopted, and the content type is checked against the content rather than against what the browser claims.

4. Missing checks on whether somebody may see something. A page checks whether somebody is logged in and then takes an identifier from the URL. Change the number and you see somebody else's data. No scanner finds that reliably and every curious user finds it in a minute.

# Places taking an identifier from the request. Every one needs the
# question: does this record belong to the logged-in user?
grep -rnE '\$_(GET|POST)\[.(id|customer|order|invoice)' src/ --include="*.php"

Three that are quick to fix

5. Dependencies with known vulnerabilities. The cheapest point on the list, because one command gives the answer and the fix is usually a version bump.

composer audit --format=table

What matters is what happens afterwards: the command belongs in the pipeline, or the list is as long again in three months. And it belongs paired with a decision about what happens on a finding that cannot be fixed immediately.

6. Error messages with an inside view. An error in production showing a stack trace with file paths, class names and sometimes credentials from the request. That is not a hole in itself, it is the manual for all the others.

php -i | grep -E "^(display_errors|expose_php|error_reporting)"
# Expected: display_errors = Off, expose_php = Off

7. Credentials in the repository. A configuration file with a password, a key in an old script, an access key in a comment. What matters: removing it from the current version is not enough, the history still contains it.

# Across the whole history, not just the current state
git log -p --all -S 'password' -- '*.php' '*.ini' '*.yml' | head -50

# More practical: a tool that knows the patterns
docker run --rm -v "$PWD:/path" trufflesecurity/trufflehog:latest \
  git file:///path --only-verified

What gets found does not just get removed, it gets revoked first: the password changes, the key is regenerated. Anything else is tidying with a feeling of safety.

Three that need structure

8. Sessions keeping the same identifier after login. Anybody who can plant a session identifier on a user before login is logged in once that user logs in. The remedy is one line, and it is almost always missing from old code.

// Straight after the credentials check succeeds:
session_regenerate_id(true);
$_SESSION['user_id'] = $user->id;

// And in the configuration, once:
// session.cookie_httponly = 1
// session.cookie_secure   = 1
// session.cookie_samesite = Lax
// session.use_strict_mode = 1

9. Forms with no protection against foreign senders. A page that changes something on request without checking whether the request came from your own site. In a framework that is built in; in grown code it is there per form or it is not.

The rebuild is unpleasant, because it touches every form. It can be done centrally: one check covering all state-changing requests, plus an explicit list of exceptions for endpoints called from outside. That list then doubles as the overview of which places those are, and that is useful in itself.

10. Admin areas separated only by a password. The area where you can do everything hangs off the same credentials as the rest, reachable from the whole internet. No discussion about password strength helps here, a second hurdle does: a restriction to known networks, a second factor, or both.

That is the point most often postponed, because it is inconvenient and delivers nothing visible. It is at the same time the one where a single incident does the most damage.

Where credentials should sit instead is covered in Configuration and secrets for Go services.

What happens in the first week

Ten points are too many for a start. What I do in the first week when taking over such a system is four things, and all four fit in a day.

1. Run composer audit and fix the findings with known exploits immediately. That is the only category where somebody with no knowledge of the system can attack.
2. Check error output in production. One setting, five minutes, and it takes the manual away from every other hole.
3. Search the repository for credentials and revoke what turns up. Not remove, revoke.
4. Restrict the admin areas, crudely via a network restriction if need be. That takes an hour and closes the largest single risk.

The rest becomes a list with dates. It belongs in the repository, not in a presentation, and every point gets a date and a name.

The same exercise on the infrastructure side is covered in Cleaning up IAM in a grown AWS account.

What this list does not deliver

Three limits belong with it, so that nobody takes away the wrong kind of confidence.

It is not complete. These are the ten I find most often. A system can satisfy all ten and still have a hole coming out of its own domain logic.

It does not check the infrastructure. Open admin access on the server, databases reachable from the internet, backups in open storage: that is a review of its own, and in grown environments it regularly finds more than the code.

It does not replace a test by somebody else. A penetration test finds things you do not find yourself, because you know the system. The ten points above are what you should fix beforehand so that such a test is not busy with them.

And the framing that helps when talking to the board: these points are not an argument for a modernization. They are all individually fixable without rebuilding the system, and that is exactly how they belong treated: as operational work with dates, not as part of a project that has to be approved first. The argument for modernization is a different one.

How a step-by-step modernization works is on its own page; the list above is preparatory work there that makes sense independently.

This article belongs to a series about systems that already exist. The retrospective orders every article in it by situation.