CodeSOD: Classic WTF: One-and-a-Half-Tiered Application Design

It's a holiday in the US today, so we're reaching back into the archives. What we really need is a single function that can do it all, and by "it" we mean "ruin your life." Original --Remy

There are several types of bad code; there's lazy code, frantic code, unaware-of-a-better-way code, and aware-of-a-better-way-but-too-apathetic-to-do-it code, to name a few. Then there're amalgamations of different types of bad code.

Moshe encountered such an amalgam when his company was trying out a new delivery service. Moshe spent some time evaluating the IE-only web interface, and was curious about some JavaScript errors he was getting. Strangely, he noticed variables named dateSQL, newSQLTag, and modeSQL.

Moshe dug a little deeper, probably thinking that his suspicions couldn't possibly be correct, only to find sendLinkVal() in the page's code:

function sendLinkVal(theDate,theStatus,MainTitle,PageTitle){ var dateSQL = " AND J.JBDeliveryDate=''" + theDate + "''" var status = "" var newSQLTag ="" var PageTitle = PageTitle var MainTitle = MainTitle //alert(dateSQL) switch (theStatus){ case "Confirmed": dateSQL= "" var modeSQL = "" modeSQL = " AND (J.JBCompanyID=31337) " status = " GlobalJobStatusView AS J WHERE J.JBCollectDate='' " + theDate + "'' AND J.JBConfirmed=''Yes'' AND J.MIStatusCode<>5" + modeSQL + " AND (ISNULL(J.JBCancelled, 0) <> 1) ORDER BY Convert(int, J.MIJobID)" break; case "Unconfirmed": dateSQL= "" var modeSQL = "" modeSQL = " AND (J.JBCompanyID=31337) " status = " GlobalJobStatusView AS J WHERE J.JBCollectDate='' " + theDate + "'' AND J.JBConfirmed=''No''" + modeSQL + " ORDER BY Convert(int, J.MIJobID)" break; case "Complete": dateSQL= "" var modeSQL = "" modeSQL = " AND (J.JBCompanyID=31337) " status = " GlobalJobStatusView AS J WHERE J.JBCollectDate='' " + theDate + "'' AND J.MIStatusCode=5" + modeSQL + " ORDER BY Convert(int, J.MIJobID)" break; case "Unconformed": dateSQL= "" var modeSQL = "" modeSQL = " AND (J.JBCompanyID=31337) " status = " GlobalJobStatusView AS J WHERE J.JBCollectDate='' " + theDate + "'' AND (J.MIConformance IS NOT NULL AND J.MIConformance<>'''') " + modeSQL + " ORDER BY Convert(int, J.MIJobID)" break; case "NoDelDate": dateSQL= "" var modeSQL = "" modeSQL = " AND (J.JBCompanyID=31337) " dateSQL =" GlobalJobStatusView AS J WHERE J.JBDeliveryDate IS NULL " + modeSQL + " ORDER BY Convert(int, J.MIJobID) " break; case "Collections": // the dateSQL is not required so set it to nothing so that it // doesn't interfere with the sql being generated at the end of // the function. dateSQL= "" var modeSQL = "" modeSQL = " AND (J.JBCompanyID=31337) " status = " GlobalJobStatusView AS J WHERE J.JBCollectDate='' " + theDate + "''" + modeSQL + " ORDER BY Convert(int, J.MIJobID)" break; case "Deliveries": // the dateSQL is not required so set it to nothing so that it // doesn't interfere with the sql being generated at the end of // the function. dateSQL= "" var modeSQL = "" modeSQL = " AND (J.JBCompanyID=31337) " status = " GlobalJobStatusView AS J WHERE J.JBDeliveryDate='' " + theDate + "''" + modeSQL + " ORDER BY Convert(int, J.MIJobID)" break; case "ColAndDel": // the dateSQL is not required so set it to nothing so that it // doesn't interfere with the sql being generated at the end of // the function. dateSQL= "" var modeSQL = "" modeSQL = " AND (J.JBCompanyID=31337) " status = " GlobalJobStatusView AS J WHERE ((J.JBDeliveryDate='' " + theDate + "'') OR (J.JBCollectDate=''" + theDate + "''))" + modeSQL + " ORDER BY Convert(int, J.MIJobID)" break; case "Subcontractor": // the dateSQL is not required so set it to nothing so that it // doesn't interfere with the sql being generated at the end of // the function. dateSQL= "" var modeSQL = "" modeSQL = " AND (J.JBCompanyID=31337) " status = " JobAndLoadView AS J WHERE (J.JBDeliveryDate='' " + theDate + "'') " + modeSQL + " ORDER BY Convert(int, J.MIJobID)" break; case "Cancelled": // the dateSQL is not required so set it to nothing so that it // doesn't interfere with the sql being generated at the end of // the function. dateSQL= "" var modeSQL = "" modeSQL = " AND (J.JBCompanyID=31337) " status = " GlobalJobStatusView AS J WHERE (J.JBCollectDate=='' " + theDate + "'') " + modeSQL + " AND ISNULL(J.JBCancelled, 0) = 1 ORDER BY Convert(int, J.MIJobID)" break; default : status =""; } newSQLTag = dateSQL + status; document.all.hiddenForm.linkVal.value = newSQLTag; document.all.hiddenForm.PageTitle.value = PageTitle document.all.hiddenForm.MainTitle.value = MainTitle document.all.hiddenForm.submit(); //alert(newSQLTag) }

Moshe could replace his customer ID with any other and access customer data, and for that matter, to modify or delete whatever he wanted. He could add or remove columns to tables. He could possibly even change permissions, add his own database user and deny all other users access.

Shocked, Moshe called the delivery service, who got him in touch with the developer of the system. This developer was equally shocked to learn that it was even possible to view a web page's JavaScript code, let alone that his architecture was open to SQL injection attacks from virtually any angle. He took immediate and decisive action; all queries were moved to the .NET backend.

Of course, the queries still didn't use parameters and are therefore still open to SQL injection, but now it takes slightly more effort to hack.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.