When Simon asked us to consider this code from his predecessor's custom-built PHP CMS, we weren't terribly impressed:

$rs = new RecordSet("SELECT * FROM moduleData WHERE moduleID = '".$moduleID."' ORDER BY displayOrder ASC");

Since that code just selects a single record by its primary key, the only thing wrong with it is the redundant ORDER BY clause. But that wasn't all. Simon leaned forward across the table, his face made sinister by the single, flickering light bulb we make every would-be submitter sit under (TDWTF policy), and he whispered, "Wouldn't you like to know about the field in moduleData called SQLCode?"

We should have known better, Dear Reader. You would have known better. You would have known not to ask, not to take Simon's hand and follow him down the rickety, rusty spiral staircase into madness.

    SELECT 1 as active, modulePR.*, DATE_FORMAT(createDate, '%M %Y') as groupBy FROM modulePR WHERE ".is_published($moduleID, 'modulePR', SITE_ID)." AND archiveStatus ".(ARCHIVE_MODE ? " =1" : " <>1")."
    SELECT 0 as active, modulePR_old.*, DATE_FORMAT(createDate, '%M %Y') as groupBy FROM modulePR_old WHERE ".is_published($moduleID, 'modulePR_old', SITE_ID)." AND archiveStatus ".(ARCHIVE_MODE ? " =1" : " <>1")." GROUP BY linkUID
    ORDER BY active DESC, lastEditedDate desc
) as pr1
) as prSorted
ORDER BY DATE_FORMAT(createDate,'%Y%m') DESC, itemTitle

Yes, that is a SQL string with PHP embedded in it, being stored in a database table. And, as Simon was quick to point out, that is_published() function returns even more SQL that makes up the first part of the WHERE clause. At this point, we'd learned our lesson. The lightbulb had flickered out while Simon described the monstrosity, and now his face was lost in shadow. He seemed to be chuckling to himself, quietly. He seemed to know, as we knew, that we were duty-bound to hear the fate of the SQL string. Praying it would just be fed to a simple eval() call, we kept recording—for you, Dear Reader. We did this for you...

while(!$rs->EOF()) {
    $moduleData['sql'] = [email protected]("return ".$rs->field("SQLCode").";") ? [email protected]("return \"".$rs->field("SQLCode")."\";") ? false : @eval("return \"".$rs->field("SQLCode")."\";") : @eval("return ".$rs->field("SQLCode").";");
    // ...snip a dozen or so more fields to build the $moduleData array, several of which contain similar thing to the above
    $rs2 = new RecordSet($moduleData['sql']);
    while(!$rs2->EOF()) {
        // ...

Simon was laughing openly now, the atonal cackle of the truly lost. With the last of our sanity we heard him say, "The SQL/PHP blob from the DB is eval()ed to determine whether it's valid PHP code. In this example it isn't, so the eval fails, so it's eval()ed again with quotes around it to determine whether it's a valid PHP string-with-embedded-PHP-code. If either of these tests succeeds, the successful eval() is run one more time to get a value to put into the $moduleData[] array."

He went on, as though unable to stop, "If neither eval() works, or if they just return a falsey value like zero or the empty string, moduleData['sql'] gets set to false. False isn't a valid SQL string, so you'd think passing it into a new RecordSet without any further error checks might be a problem, but, no! The RecordSet class fails silently on SQL errors, and just sets EOF to true."

That's all we got out of Simon, who would do nothing further but mutter "GROUP BY with no aggregate" over and over. Weep for him, Dear Reader, for he has surely glimpsed an abyss. And shed a tear for yourself, for now you have, too.

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