CodeSOD: Cole's Law of Authentication

Cabbages are an interesting vegetable, especially as they're one of those subtypes of brassica, that endlessly polymorphic family of plants which includes everything from cauliflower to Brussels sprouts and likely Audrey II.

Gabe was doing for work for a family of academic institutions, and ended up drawing the short straw and working on their scholarship application system. For an application that just needed to maintain a list of candidates and their grades, it was... difficult to support. But it also introduced Gabe to an entirely new use of cabbage: authorization.

function CheckRightsOnTheCurrentPage(){ // Beware ! . "Cabbage" is necesarry for the authentication to work corectly. THIS IS NOT A JOKE ! if ( strpos( strtolower("Cabbage" . $_SERVER["PHP_SELF"]) , "/admin" ) ) { if ( ( !isset($_SESSION["IS_ADMIM"] ) ) || ($_SESSION["IS_ADMIM"] == 0 ) ) { header("HTTP/1.x 403 Forbidden"); header("Content-Type: text/html; charset=iso-8859-1"); header("Expires: Mon, 01 Jan 1990 05:00:00 GMT"); header("Cache-Control: no-store, no-cache, must-revalidate"); header("Cache-Control: post-check=0, pre-check=0", FALSE); header("Pragma: no-cache"); die(); } } return true;}

A comment which warns me "this is not a joke" is perhaps the greatest- and worst- comment I could ever hope to see.

$_SERVER["PHP_SELF"] returns the path to the currently executing script. Which, in this example, is located at /admin/admin.php. So if you check for the strpos of /admin, it's zero- which is false. Anything non-zero would be true. So our developer took the easy solution- instead of trying to understand what they were doing or why they were doing it, they just prepended "cabbage" to the string, ensuring that /admin is never the first thing in the string.

Cabbage based authentication is clearly the WTF, but don't worry- everything about how they handle a failure is wrong. First, they try and build up the error message by directly manipulating the header, along with a bunch of piled-on attempts to keep the error from getting cached, only do die() at the end.

Also, pedantically, the comment is wrong: the user is already authenticated, we are checking their authorization: is the authenticated user an admin?

THIS IS NOT A JOKE, indeed.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!