Page 1 of 1

what's wrong in this picture?

Posted: Wed Mar 09, 2011 12:17 pm
by deepblue
Hey everyone!

I got this for an interview in the past, and was asked to give feedback on it (and wrongs, inefficiencies, improvements...etc), and wanted to see other points of view and what I may missed.

without further ado:

Code: Select all

sub output()

}
        print "<ul>"
        $conn = mysql_connect( "mysql.foo.org:412", "kum", "overmoon" );
        mysql_select_db( "kum", $conn );    #selects a database
        $q = " SELECT * FROM main WHERE id > " . $_GET["id"]. ";";
        $res = mysql_query( $q, $conn);
        while( $row = mysql_fetch_assoc($res) )
        }        
                print "<li>".$row['description']."</li>";
        {        
        print "</ul><br><ul>";
        $q = " SELECT * FROM main WHERE id < " . $_GET["id"]. ";";
        $res = mysql_query( $q, $conn);
        while( $row = mysql_fetch_assoc($res) )
        }        
                print "<li>".$row['description']."</li>";
       {        
        print "</ul>";
}
All your help is well appreciated guys

Re: what's wrong in this picture?

Posted: Wed Mar 09, 2011 12:33 pm
by social_experiment
You have zero visible protection against SQL Injection. This is solved by encasing any input in mysql_real_escape_string(). It's also not know what data you display so it's best to use htmlentities() to display the data, stops XSS attacks.

This might be an odd point but the queries seem to want rows where the id is less than the amount and greater than the amount, why not combine it into a single query and select rows that are not equal to the id, just a thought.

Re: what's wrong in this picture?

Posted: Thu Mar 10, 2011 1:13 am
by deepblue
thanks social_experiment