One-and-a-Half-Tiered Application Design

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.

Môshe encountered such an amalgam when his company was trying out a new delivery service. Môshe 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.

Môshe 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)
  }

Môshe 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, Môshe 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] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!