CodeSOD: Contractor's Leftovers

Вторник, 02 Ноября 2021 г. 09:30 + в цитатник

There once was a developer who had a lot of hustle. They put out a shingle as a contractor, knocked on doors, made phone calls, and targeted those small businesses that needed something a little more custom than just off-the-shelf could get, but didn't have the money to afford a larger dev shop.

And after finishing a handful of projects and building a reputation, this developer took a job at a large firm, in another town, and left a lot of unhappy customers with unfinished software behind.

This is where Graeme comes in. He got a call from a local hotel who needed their booking system finished up. It had some… colorful choices.

    $sql_search = "Select * from residence_main where Active=1 ";

    if ($req_typ_id !== "NoType")
    {
        if ($req_typ_id == "1")
        {
            $sql_search = $sql_search."And type_id1=1 ";
        }
        elseif ($req_typ_id == "2")
        {
            $sql_search = $sql_search."And type_id2=1 ";
        }
        elseif ($req_typ_id == "3")
        {
            $sql_search = $sql_search."And type_id3=1 ";
        }
        // snip
        elseif ($req_typ_id == "10")
        {
            $sql_search = $sql_search."And type_id10=1 ";
        }
    }

Instead of having a single "type" column which could be mapped as essentially an enum, and maybe use a foreign key to a type table, they instead had ten type columns. Integer columns, which were used as a boolean value.

The only good thing I can see in this is that it doesn't allow for any SQL injection attacks, so that's something anyway.

    if ($req_bed_id !== "NoBed")
    {
        $sql_search = $sql_search."And Bedrooms=$req_bed_id ";
    }

    if ($req_loc_id !== "NoLoc")
    {
        $sql_search = $sql_search."And loc_id=$req_loc_id ";
    }

    if ($req_key_id !== " Keywords")
    {
        $sql_search = $sql_search."And res_desc LIKE '%".$req_key_id."%' ";
    }

Ah, there we go. I was worried for a moment that we wouldn't have a SQL injection vulnerability. Of course, even with this clear exploit, Graeme has worse news:

The query string is used without any escaping, but it would really not be necessary to bother with SQL injection. Anyone who navigated to a special super-secret URL (added the path "/mydblak" to the domain name) they would find themselves in a rather old version of PHPMyAdmin - no password or other inconvenience required.