CodeSOD: The God Page

Mike inherited a data-driven application. Once upon a time, it started out pretty well architected. Yes, it was in PHP, but with good architecture, solid coding practices, and experienced developers, it was simple and easy to maintain.

Time passed. The architects behind it left, new developers were hired, management changed over, and it gradually drifted into what you imagine when you hear "PHP app" in 2019.

Mike's task was to start trying to clean up that mess, and that started all the way back in the database layer with some normalization. From there, it was the arduous task of going through the existing data access code and updating it to use the refined model objects.

While there was a mix of "already uses a model" and "just a string" SQL statements in the code, there was a clear data-access layer, and almost all of the string-based queries actually used parameters. At least, that was true until Mike took a look at the "god" page of the application.

You know how it is. Thinking through features and integrating them into your application is hard and requires coordinating with other team members. Slapping together a page which can generate any arbitrary SQL if you understand how to populate the drop-downs and check boxes correctly is "easy"- at least to start. And by the time it gets really hard, you've already created this:

$numIDs=0;$statusdb1 = "";$statusdb2 = "";if ($cstatus=='inactive') {$statusdb1 = "WHERE customer_status='0'";$statusdb2 = "AND customer_status='0'";$childIDs.=" and ";} elseif ($cstatus=='active') {$statusdb1 = "WHERE customer_status='1'";$statusdb2 = "AND customer_status='1'";$childIDs.=" and ";}$childIDs.="(";if($isParent) {$str="select ChildID from customer_parent_child where ParentID=$custID order by ChildID;";$result=$database->query($str);if(count($result) > 0) {$row = $result[0];$IDs[$numIDs]=$row['ChildID'];$childIDs.="custID='$IDs[$numIDs]'";$numIDs++;}foreach ($result as $row){$IDs[$numIDs]=$row['ChildID'];$childIDs.=" or custID='$IDs[$numIDs]'";$numIDs++;}}$childIDs.=")";if($numIDs==0) {$childIDs=" and custID=''";}//echo "childIDs:$childIDs<br />";if ($charge) { $chargedb = " customer_child=0";}if(isset($_POST['cmpSearchFname'])){$search=$_POST['fname'];if (!$_POST['fname']) {$search=$_GET['fname'];}if(isset($search)){if(($childIDs !== "") && ($numIDs >0)) {$childIDs=" and $childIDs";}$str="select * from customer where customer_firstname like '%$search%' $statusdb2 $childIDs order by customer_firstname;";} else{if($statusdb1=="") {$statusdb1="where ";}$str="select * from customer $statusdb1 $childIDs order by customer_firstname;";}} elseif (isset($_POST['cmpSearchcustID'])){$search=$_POST['custID'];if (!$_POST['custID']) {$search=$_GET['custID'];}if(isset($search)){if(($childIDs !== "") && ($numIDs >0)) {$childIDs=" and $childIDs";}$str="select * from customer where custID like '%$search%' $statusdb2 $childIDs order by custID;";}else{if($statusdb1=="") {$statusdb1="where ";}$str="select * from customer $statusdb1 $childIDs order by custID;";}} elseif (isset($_POST['cmpSearchLname'])){$search=$_POST['lname'];if (!$_POST['lname']) {$search=$_GET['lname'];}if(isset($search)){if(($childIDs !== "") && ($numIDs >0)) {$childIDs=" and $childIDs";}$str="select * from customer where customer_lastname like '%$search%' $statusdb2 $childIDs order by customer_lastname;";}else{if($statusdb1=="") {$statusdb1="where ";}$str="select * from customer $statusdb1 $childIDs order by customer_lastname;";}} elseif(isset($_POST['cmpSearchCmpName'])){$search=$_POST['cmpName'];if (!$_POST['cmpName']) {$search=$_GET['cmpName'];}if(isset($search)) {if(($childIDs !== "") && ($numIDs >0)) {$childIDs=" and $childIDs";}$str="select * from customer where customer_cmp_name like '%$search%' $statusdb2 $childIDs order by customer_cmp_name;";} else {if($statusdb1=="") {$statusdb1="where ";}$str="select * from customer $statusdb1 $childIDs order by customer_cmp_name;";}} elseif (isset($_POST['cmpSearchPhone'])){$search1=$_POST['phone1'];if (!$_POST['phone1']) {$search1=$_GET['phone1'];}$search2=$_POST['phone2'];if (!$_POST['phone2']) {$search2=$_GET['phone2'];}$search3=$_POST['phone3'];if (!$_POST['phone3']) {$search3=$_GET['phone3'];}$phonesearch = "FALSE";if(isset($search1)){$search=$search1;$phonesearch = "TRUE";}if(isset($search2)){$search.=$search2;$phonesearch = "TRUE";}if(isset($search3)){$search.=$search3;$phonesearch = "TRUE";}if($phonesearch == "TRUE"){if(($childIDs !== "") && ($numIDs >0)) {$childIDs=" and $childIDs";}$str="select * from customer where customer_contact_phone1 like '%$search%' $statusdb2 $childIDs order by customer_contact_phone1;";} else {if($statusdb1=="") {$statusdb1="where ";}$str="select * from customer $statusdb1 $childIDs order by customer_contact_phone1;";}} elseif(isset($_POST['cmpSearchDID'])){$search1=$_POST['DID1'];if (!$_POST['DID1']) {$search1=$_GET['DID1'];}//echo "DID1: ".$search1."<br>";$search2=$_POST['DID2'];if (!$_POST['DID2']) {$search2=$_GET['DID2'];}//echo "DID2: ".$search2."<br>";$search3=$_POST['DID3'];if (!$_POST['DID3']) {$search3=$_GET['DID3'];}//echo "DID3: ".$search3."<br>";$DIDsearch = "FALSE";if(isset($search1)){$search=$search1;$DIDsearch = "TRUE";}if(isset($search2)){$search.=$search2;$DIDsearch = "TRUE";}if(isset($search3)){$search.=$search3;$DIDsearch = "TRUE";}if($DIDsearch == "TRUE"){//echo "DID: ".$search."<br>";$str="select custID from DID where DID like '%$search%' $childIDs;";$result=$database->query($str);$row = $result[0];$numRows = count($result);if($numRows > 0){$custSearch=$row['custID'];if(($childIDs !== "") && ($numIDs >0)) {$childIDs=" and $childIDs";}$str="select * from customer where custID like '%$custSearch%' $statusdb2 $childIDs;";} else {if(($childIDs !== "") && ($numIDs >0)) {$childIDs=" and $childIDs";}$str="select * from customer where customer_cmp_name like \"\" $statusdb2 $childIDs order by customer_cmp_name";}} else {if($statusdb1=="") {$statusdb1="where ";}$str="select * from customer $statusdb1 $childIDs order by customer_cmp_name";}} else {if($statusdb1=="") {$statusdb1="where ";}$str="select * from customer $statusdb1 $childIDs order by customer_cmp_name";}$result=$database->query($str);

Mike writes:

I expect that there are SQL injection vulnerabilities in here somewhere. I just can't read through this well enough to find them.

I read through it. There are definitely SQL Injection errors. I still couldn't tell you exactly what all this does, but it definitely has SQL injection errors. Also, if you don't POST your query parameters, it'll pull them from the GET, so, y'know, do either. It doesn't matter. Nothing matters.

Mike adds:

I did the only thing I could think of to do with this code - sumbit it to TheDailyWTF.

I hope you also deleted it and removed any trace of it from source control history, but submitting here first was the right call.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!