Secure your PHP using one include file!



  • Hello, I have been browsing TDWTF for some time, I just haven't bothered to register. But today I was looking up some information about PHP security, and I came across this gem:

     http://www.devarticles.com/c/a/MySQL/Security-and-Sessions-in-PHP/

    I just had to post it here. Count the WTFs, I stopped somewhere at around 10.



    1. Using yellow marking over all the sourcecode
    2. Foregoing the built in session system for no good reason
    3. Hungarian notation
    4. Leaving the storage engine setting in the sql dump.
    5. Using an obsolete password hashing function.
    6. Not getting the case correct for the method attribute for the form element
    7. Blindly echoing out $refer
    8. Using register globals
    9. Using the outdated mysql extension
    10. Using a syntacticly invalid select query
    11. Generating the id with said query
    12. Not escaping the username or password in said query
    13. Using _affected_rows for a select query.
    14. Using HTTP_COOKIE_VARS instead of _COOKIES
    15. Not terminating execution after redirecting, making all the effort useless.
    16. Hardcoding the mysql login in each script that needs a connection
    17. Not escaping input in select query for login check.
    18. Using _affected_rows for a select, again.
    19. Pretending that require is a function.


  • @henke37 said:

    Pretending that require is a function.
     

    i do this one too, why is it bad? and why php allows it, if it's bad?



  • @SEMI-HYBRID code said:

    why php allows it, if it's bad?
     

    It's not about good, bad, or allowing things. It just parses to a keyword with a parenthesised statement, and as such is syntactically valid, even if it's redundant. It's just like the numbnuts who write return(val) in javascript.

    The more interesting thing is that it's listed in the function list, with parens, and most commenters on the page omit the parens.



  • @dhromed said:

    The more interesting thing is that it's listed in the function list, with parens, and most commenters on the page omit the parens.
    All the language constructs seem to be listed in the function list, just not so often with parenthesis. There are far more interesting (read: trivial and pedantic) things about PHP’s language constructs anyway, like for example the fact that you can return a value from an include, or that you can pass an unlimited number of arguments to echo. When I gave a code test and asked people to identify wrong things, I think every single one said that using commas was invalid and that they should be dots. It’s knowing obscure stuff like that that makes me a terrible person.



  • 19. Pretending that XHTML is HTML (not really a WTF, but it irritates me anyway)
    20. Failing to close the <input
    21. DOCTYPEs are for people who want a predictable response to CSS, and should always be omitted for maximum IE 4 compatibility. Yeah.
    22. <pile><of><arse>Community server</arse></of></pile>

    But hey, at least it uses require instead of include.



  • Am I the only one noticing that this article is from 2003?

    Did $_COOKIES even exist back then?



  • @Aerendel said:

    Am I the only one noticing that this article is from 2003?

    Did $_COOKIES even exist back then?

     

    You're not the only one. Of course $_COOKIES existed all the way back then, but I'm not sure if the mysql extension would have been 'outdated' then.  And I can't be bothered finding out.



  •  This is a tutorial from 7 years ago. TWTF is that you are reading it!!!!



  • 22. <font face="Verdana" size="2">MySql -- it's called MySQL. Also not capitalizing SQL keywords like everyone else, making the queries really hard to find and read. </font>

    23. Failing to indent any of the code.

    <font size="2"><font face="Verdana">24. "</font></font><font face="Verdana" size="2">You can have the login expire by adding a logged-in date field to tblUsers. You would need to check that date against your "timeout" in incSession.php." -- That is not how timeouts work. You store the date of last activity, not of login.</font>

    25. "<font face="Verdana" size="2">To generate the GUID or session ID, you need to generate a unique number and hash it to protect would-be hackers from brute-forcing their way into your application." -- That's not how brute force works either.</font>

    26. "<font face="Verdana" size="2">I believe this method to be one of the most portable and easily customized methods available." -- I believe you to be a complete moron.</font>

     

    Also, I always hated the idea of PHP's implicit string interpolation for stuff like "I have $kittencount kittens!"  but I suppose that's more style than a WTF.



  • @scgtrp said:

    Also, I always hated the idea of PHP's implicit string interpolation for stuff like "I have $kittencount kittens!"  but I suppose that's more style than a WTF.
    It's not a PHP-centric thing; PHP got it from Perl.



  • @bstorer said:

    @scgtrp said:

    Also, I always hated the idea of PHP's implicit string interpolation for stuff like "I have $kittencount kittens!"  but I suppose that's more style than a WTF.
    It's not a PHP-centric thing; PHP got it from Perl.

    Perl got it from shell scripting.



  • @scgtrp said:

    22. <font size="2" face="Verdana">MySql -- it's called MySQL. Also not capitalizing SQL keywords like everyone else, making the queries really hard to find and read. </font>
     

    I lowercase SQL queries because I find it much, much easier to read. Of course, I also use a text editor with auto-completion and syntax highlighting for SQL.

    I think the uppercase rule is ancient dreck from mainframes that needs to go away. Uppercase words are no easier to read than lowercase ones, and you should be using syntax highlighting if you have problems telling SQL keywords apart from (say) table names.



  • @blakeyrat said:

    I think the uppercase rule is ancient dreck from mainframes that needs to go away. Uppercase words are no easier to read than lowercase ones, and you should be using syntax highlighting if you have problems telling SQL keywords apart from (say) table names.
    I wish more people followed the "capitalize reserved words but not field/table names"  for the following reasons:

    1. When coworkers email or IM me SQL code, it's not colored, and I would have to copy it into GVIM or SQL developer to get the syntax coloring
    2. .sql files open in notepad by default 
    3. One particular coworker (who got her start in mainframes) capitalizes the reserved words and the field/table names.  Even with syntax coloring, it's a mess to read.  Cognos does even worse when it generates SQL for you.  It creates an alias for each table, like PROJECT_FACT_TABLE as "PROJECT_FACT_TABLE".  so you get things like WHERE "PROJECT_FACT_TABLE".MEASURE > "PROJECT_DIMENSION_TABLE".LIMIT.  It makes me want to scream.
    On another note, I'll kill the blithering idiot that thinks that a, b, and c are acceptable table aliases.


  • @blakeyrat said:

    I lowercase SQL queries because I find it much, much easier to read. Of course, I also use a text editor with auto-completion and syntax highlighting for SQL.

    My editor has highlighting too, when the SQL is in a separate file - however, that's rarely true except for database dumps. In this case the SQL is embedded in the PHP using normal strings - no special syntax at all, so my editor doesn't know to highlight the contents of the string as SQL.

     

    (And no, highlighting every string as SQL isn't an acceptable solution. echo 'Please select the state where you live:'; )



  • @snover said:

    There are far more interesting (read: trivial and pedantic) things about PHP’s language constructs anyway, like for example the fact that you can return a value from an include, or that you can pass an unlimited number of arguments to echo. When I gave a code test and asked people to identify wrong things, I think every single one said that using commas was invalid and that they should be dots. It’s knowing obscure stuff like that that makes me a terrible person.

    Perl's print and Ruby's puts also take variadic arguments.  I guess it's maybe a bit obscure, but I would expect any PHP developer I was considering to know it.  If they don't, then they clearly don't have a lot of experience in PHP or in similar scripting languages.



  • @belgariontheking said:

  • When coworkers email or IM me SQL code, it's not colored, and I would have to copy it into GVIM or SQL developer to get the syntax coloring
  •  

    If they're using an editor that does color-coding, it should still be color-coded in the email right? Or are you one of those crazies who uses Plain Text emails and then complains when stuff is sent as plain text? (Or are your co-workers idiots who don't use color-coded editors?)

    @belgariontheking said:

    .sql files open in notepad by default 

    Two points here:

    1. SQL Server Management Studio takes ownership of .sql files by default. Why doesn't your editor?

    2. This takes like 2 seconds to change, you lazy bastard :P

    @belgariontheking said:

    One particular coworker (who got her start in mainframes) capitalizes the reserved words and the field/table names.  Even with syntax coloring, it's a mess to read.  Cognos does even worse when it generates SQL for you.  It creates an alias for each table, like PROJECT_FACT_TABLE as "PROJECT_FACT_TABLE".  so you get things like WHERE "PROJECT_FACT_TABLE".MEASURE > "PROJECT_DIMENSION_TABLE".LIMIT.  It makes me want to scream.

    That's just ... upsetting. I don't think I could work with a person like that.

    @belgariontheking said:

    On another note, I'll kill the blithering idiot that thinks that a, b, and c are acceptable table aliases.

    I might be on your hit-list. I alias tables by the first letter of their full name... "events.eventid" = e.eventid for example.



  • @scgtrp said:

    @blakeyrat said:

    I lowercase SQL queries because I find it much, much easier to read. Of course, I also use a text editor with auto-completion and syntax highlighting for SQL.

    My editor has highlighting too, when the SQL is in a separate file - however, that's rarely true except for database dumps. In this case the SQL is embedded in the PHP using normal strings - no special syntax at all, so my editor doesn't know to highlight the contents of the string as SQL.

     

    (And no, highlighting every string as SQL isn't an acceptable solution. echo 'Please select the state where you live:'; )

     

    It's 2010. What's your excuse for not using stored procedures by now?



  • @morbiuswilters said:

    Perl's print and Ruby's puts also take variadic arguments.  I guess it's maybe a bit obscure, but I would expect any PHP developer I was considering to know it.  If they don't, then they clearly don't have a lot of experience in PHP or in similar scripting languages.
    Ah, but you see, unlike echo, PHP’s print function only accepts one argument. (And also returns “1”. Fuck if I know why.)



  • @belgariontheking said:

    On another note, I'll kill the blithering idiot that thinks that a, b, and c are acceptable table aliases.
    Better than t, t2, t3, t4, etc.



  • @snover said:

    @morbiuswilters said:

    Perl's print and Ruby's puts also take variadic arguments.  I guess it's maybe a bit obscure, but I would expect any PHP developer I was considering to know it.  If they don't, then they clearly don't have a lot of experience in PHP or in similar scripting languages.
    Ah, but you see, unlike echo, PHP’s print function only accepts one argument. (And also returns “1”. Fuck if I know why.)

    The better question is, why does print even exist?  And who even uses it?



  • @belgariontheking said:

    When coworkers email or IM me SQL code, it's not colored, and I would have to copy it into GVIM or SQL developer to get the syntax coloring

    Agreed.
    @belgariontheking said:
  • .sql files open in notepad by default 

  • ... or you only have access to the CLI tool for DB admin and thus have no highlighting what-so-ever when writting queires.
    @belgariontheking said:
    On another note, I'll kill the blithering idiot that thinks that a, b, and c are acceptable table aliases.

    Of course, X, Y and Z are much superior aliases.



  • @morbiuswilters said:

    The better question is, why does print even exist?  And who even uses it?
    It's so idiots waste their time doing benchmarks of echo vs print.



  •  I once had a coworker who liked to put "print" everywhere in his code when the others used "echo". He said it was his coding style.

    I left that company.



  • @blakeyrat said:

    It's 2010. What's your excuse for not using stored procedures by now?

    I was at a .Net conference late last year and I had the misfortune of going to one presentation by some pompous guy who supposedly was a luminary in the .Net world. He spent quite a bit of time railing on how SPs were evil and you should do everything inside your own App and how inherently evil DB admins were. I sat there dumbfounded at the thought of turning a DBMS into a dumb file store and losing all the high level functionality that is a DBMS. I can only think that this guy was molested by a DB admin at a young age.



  • @OzPeter said:

    @blakeyrat said:
    It's 2010. What's your excuse for not using stored procedures by now?

    I was at a .Net conference late last year and I had the misfortune of going to one presentation by some pompous guy who supposedly was a luminary in the .Net world. He spent quite a bit of time railing on how SPs were evil and you should do everything inside your own App and how inherently evil DB admins were. I sat there dumbfounded at the thought of turning a DBMS into a dumb file store and losing all the high level functionality that is a DBMS. I can only think that this guy was molested by a DB admin at a young age.

     

    I get the impression that that's what all MySQL coders do.

    To be fair, if you're working in something like Amazon SimpleDB or SQL Lite, that's an ok attitude to have. Otherwise, you might as well not have a DB at all.



  • @OzPeter said:

    @blakeyrat said:
    It's 2010. What's your excuse for not using stored procedures by now?

    I was at a .Net conference late last year and I had the misfortune of going to one presentation by some pompous guy who supposedly was a luminary in the .Net world. He spent quite a bit of time railing on how SPs were evil and you should do everything inside your own App and how inherently evil DB admins were. I sat there dumbfounded at the thought of turning a DBMS into a dumb file store and losing all the high level functionality that is a DBMS. I can only think that this guy was molested by a DB admin at a young age.

    Oh, he must have been recommending you use the giant anti-pattern that is LINQ. First, they spend 10 years teaching us "don't hard code SQL, since then you need a recompile to change them - use stored procs", now they teach us "don't use stored procs, since then it's harder to code - use LINQ in your application layer".



  • @blakeyrat said:

    I lowercase SQL queries because I find it much, much easier to read. Uppercase words are not easier to read than lowercase ones.
     

    Lowercase FTW. I don't see a compelling reason to write uppercase SQL. There's a very compelling argument to keep it all lowercase, though: it's not annoying to type because you don't have to switch back between (holding down shift || tapping Caps Lock) and just typing normally.

    @belgariontheking said:

    When coworkers email or IM me SQL code, it's not colored, and I would have to copy it into GVIM or SQL developer to get the syntax coloring
     

    :care

    You have to copy it anyway if you're going to use the code seriously.

    @belgariontheking said:

    .sql files open in notepad by default 

    Then associate them with your editor.

    On my machine, they're associated with SQL Studio Express.

    [quote user="belgariontheking"]On another note, I'll kill the blithering idiot that thinks that a, b, and c are acceptable table aliases.[/quote]

    +10

    I make up a three- or two-letter code based on the table name. Makes for a great shorthand.

    @blakeyrat said:

    If they're using an editor that does color-coding, it should still be color-coded in the email right?
     

    Depends. VS does it, and maybe Eclipse, but I know of no other editor that retains colouring when copypasting.

    @snover said:

    @belgariontheking said:
    On another note, I'll kill the blithering idiot that thinks that a, b, and c are acceptable table aliases.
    Better than t, t2, t3, t4, etc.
     

    I think  a, b, and c are just as bad as t1 t2 t3.

     

     



  • @dhromed said:

    There's a very compelling argument to keep it all lowercase, though: it's not annoying to type because you don't have to switch back between (holding down shift || tapping Caps Lock) and just typing normally.

    It's not hard at all, I Just hold my right pinky on the right shift key and type normally. Even with syntax hilighting it looks better, more mature and dignified like an ancient piece of COBOL that nobody would dare touch. Lowercase SQL just looks like a quick hack that someone typed, um quickly.

    Also I put semicolons to the end of my SQL statements.

    And prefix NVARCHAR string literals with N.



  • @SlyEcho said:

    Lowercase SQL just looks like a quick hack that someone typed, um quickly.
     

    Excellent argument. I'm convinced.

    @SlyEcho said:

    I Just hold my right pinky on the right shift key and type normally.

    We don't all have hypermobile hands. Holding my pinky there permantly while typing is not conducive to comfort.

     



  • @dhromed said:

    Lowercase FTW. I don't see a compelling reason to write uppercase SQL. There's a very compelling argument to keep it all lowercase, though: it's not annoying to type because you don't have to switch back between (holding down shift || tapping Caps Lock) and just typing normally.
     

    QFT

    @dhromed said:

    Depends. VS does it, and maybe Eclipse, but I know of no other editor that retains colouring when copypasting.

    TOAD, probably the most popular* SQL tool for Oracle, does that, too. Not that it matters that much.

    IMO, uppercased SQL statements look like COBOL. I've even written a little tool to lowercase PL/SQL files (preserving string literals, quoted identifiers and comments), for those PL/SQL packages written by coworkers that uppercase keywords and identifiers.

     

    * popular as in: used by many people. not as in: loved by many people



  • @Lingerance said:

    @morbiuswilters said:
    The better question is, why does print even exist?  And who even uses it?
    It's so idiots waste their time doing benchmarks of echo vs print.

    Well, I'm afraid the manual has the answer... http://php.net/manual/en/function.echo.php

    echo() is not actually a function (it is a language construct), so you are not required to use parentheses with it. echo() (unlike some other language constructs) does not behave like a function, so it cannot always be used in the context of a function. Additionally, if you want to pass more than one parameter to echo(), the parameters must not be enclosed within parentheses.
    But no, that's not TRWTF.

    The fact that "Not a function" is described in the Functions Manual might be.
    But I, personally, vote for this:

    echo() (unlike some other language constructs) does not behave like a function,
    in context of http://php.net/manual/en/function.print.php
    print() is not actually a real function (it is a language construct)
    which DOES behave like a function.



  • @bannedfromcoding said:

    @Lingerance said:
    @morbiuswilters said:
    The better question is, why does print even exist?  And who even uses it?
    It's so idiots waste their time doing benchmarks of echo vs print.

    Well, I'm afraid the manual has the answer... http://php.net/manual/en/function.echo.php

    echo() is not actually a function (it is a language construct), so you are not required to use parentheses with it. echo() (unlike some other language constructs) does not behave like a function, so it cannot always be used in the context of a function. Additionally, if you want to pass more than one parameter to echo(), the parameters must not be enclosed within parentheses.

    Sorry you got confused. I was actually refering to a post that used to be on one of the echo/print pages where someone actually benchmarked the difference.


  • @blakeyrat said:

    If they're using an editor that does color-coding, it should still be color-coded in the email right? Or are you one of those crazies who uses Plain Text emails and then complains when stuff is sent as plain text? (Or are your co-workers idiots who don't use color-coded editors?)
    No.  TOAD (One of the worst non-free programs I have ever seen) is the only one that I know of that does this, and AFAIK it only does it in word documents and possibly RTF emails.  Fuck Rich Text emails though.  SQL Developer doesn't, GVIM sure as hell doesn't.@blakeyrat said:

    @belgariontheking said:

    .sql files open in notepad by default 

    Two points here:

    1. SQL Server Management Studio takes ownership of .sql files by default. Why doesn't your editor?

    I set Windows to open .sql files in GVIM.  I wouldn't do it in any heavier app than a text editor.

    @blakeyrat said:

    2. This takes like 2 seconds to change, you lazy bastard :P
    Yeah.  I did that.

    @blakeyrat said:

    That's just ... upsetting. I don't think I could work with a person like that.
    Luckily, I don't have to work with SQL from that mainframe lady much, but Cognos' SQL makes me rage.@blakeyrat said:
    I might be on your hit-list. I alias tables by the first letter of their full name... "events.eventid" = e.eventid for example.
    That is acceptable because it has something to do with the actual table and what it is.  I don't want to have to remember that a is my main fact table, b is the date table, and c thru f are copies of the lookup table.  fuck that.  Even e1, e2, e3 for three different copies of the event table would be non murder-worthy, though I would prefer e_birthday, e_dethday, and e_wedding_day.



  •  Ah. Our company is pretty Microsoft-centric, so I guess I just assume other solutions are as non-shitty as Microsoft solutions. (Although I still don't get why your editors don't copy the style data along with the text-- what is this, 1985?)

    You are right that SSMS is pretty damned heavy, but on the other hand I never close it.

    I'm glad my query aliasing scheme is murder-free.



  • Oh yeah, the other thing I forgot to mention.  The Cognos-generated SQL is usually sent to me in .txt files.  RAAAAAGE!



  • @blakeyrat said:

    I still don't get why your editors don't copy the style data along with the text-- what is this, 1985?
     

    Visual Studio does it.



  • @dhromed said:

    @blakeyrat said:

    I still don't get why your editors don't copy the style data along with the text-- what is this, 1985?
     

    Visual Studio does it.

     

     Eclipse does it, but manages to screw it up by copying the highlight color as well (but not the foreground text color).  So, by default it pastes black text on a dark-blue background that is impossible to actually read.



  • @powerlord said:

    Eclipse does it, but manages to screw it up by copying the highlight color as well
     

    o_O



  • @dhromed said:

    @powerlord said:

    Eclipse does it, but manages to screw it up by copying the highlight color as well
     

    o_O

     

    A Java app making elementary GUI errors? SAY IT AIN'T SO!



  • @blakeyrat said:

    A Java app making elementary GUI errors? SAY IT AIN'T SO!
     

    Right, I forgot about that. Thanks.



  • @powerlord said:

    @dhromed said:

    @blakeyrat said:

    I still don't get why your editors don't copy the style data along with the text-- what is this, 1985?
     

    Visual Studio does it.

     

     Eclipse does it, but manages to screw it up by copying the highlight color as well (but not the foreground text color).  So, by default it pastes black text on a dark-blue background that is impossible to actually read.

    I don't know what broken, retarded version of Eclipse you're using, but 3.4 copies styling without any such problems.


  • @blakeyrat said:

    You are right that SSDS is pretty damned heavy, but on the other hand I never close it.

     

    FTFY.



  • @bstorer said:

    @powerlord said:

    @dhromed said:

    @blakeyrat said:

    I still don't get why your editors don't copy the style data along with the text-- what is this, 1985?
     

    Visual Studio does it.

     

     Eclipse does it, but manages to screw it up by copying the highlight color as well (but not the foreground text color).  So, by default it pastes black text on a dark-blue background that is impossible to actually read.

    I don't know what broken, retarded version of Eclipse you're using, but 3.4 copies styling without any such problems.
     

    I think he's talking about when you copy and paste it into an email.  It has nothign to do with eclipse, but with MS Outlook.  Sometimes when I copy paste code into an email I have to select "Keep Text Only" and other times it shows the code just as it looks in Eclipse.  I think the problem is with Outlook though and not Eclipse.

     

    When I copy paste simple lines like:

     InvCatTranslationEntry l_answer = null;

     I get the black text with a dark blue background, but when I copy paste something like:

    if ( null == l_answer )
    {
          //something
    }

    Everything looks as it does in Eclipse.  And I use Ganymede for reference.



  • @blakeyrat said:

    @OzPeter said:

    @blakeyrat said:
    It's 2010. What's your excuse for not using stored procedures by now?

    I was at a .Net conference late last year and I had the misfortune of going to one presentation by some pompous guy who supposedly was a luminary in the .Net world. He spent quite a bit of time railing on how SPs were evil and you should do everything inside your own App and how inherently evil DB admins were. I sat there dumbfounded at the thought of turning a DBMS into a dumb file store and losing all the high level functionality that is a DBMS. I can only think that this guy was molested by a DB admin at a young age.

     

    I get the impression that that's what all MySQL coders do.

    To be fair, if you're working in something like Amazon SimpleDB or SQL Lite, that's an ok attitude to have. Otherwise, you might as well not have a DB at all.

     

    Isn't this recommended practice for RoR programmers?



  • @Watson said:

    Isn't this recommended practice for RoR programmers?
     

    I'm not sure if it's "recommended", but it's one of those things where it makes it *so much easier* if your data isn't normalized, since you can code up really dumb INSERT functions. I haven't played with RoR in years, so my memory might be wrong.

    I've worked with a couple object storage libraries in PHP, and whenever you ask the developers of the library how to cope with joined tables, they always give some bullshit work-around, like "query both and write a loop in code to join the data." Uh... no. One of them actually supported joins, but not DB-native joins, meaning it just manually looked up the IDs and added it to the WHERE clause of the second query. Retards. (It also stored every string, IIRC, as binhex to avoid SQL insertion errors. That's about the most retarded way ever to avoid insertion errors... and makes your database tables inpossible to ad-hoc query to-boot. Retards.)

    The funny thing is that I went to a cheap local college, and I got a pretty good education in relational databases. Are these people all sleeping through that class? Or just idiots?


  • Discourse touched me in a no-no place

    @blakeyrat said:

    The funny thing is that I went to a cheap local college, and I got a pretty good education in relational databases. Are these people all sleeping through that class? Or just idiots?
    Was that a rhetorical question?



  • @snover said:

    @belgariontheking said:
    On another note, I'll kill the blithering idiot that thinks that a, b, and c are acceptable table aliases.
    Better than t, t2, t3, t4, etc.
    And, in turn, t1, t2, t3, t4 etc is better than [picks random query] T624488, T637756, T637888, T638592, T626875, T625885.


Log in to reply