SitePoint Sponsor

User Tag List

Results 1 to 3 of 3
  1. #1
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,764
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)

    How much checking is enough?

    This topic has been a dilemma in my mind for some time...

    If Code-Block-3 depends on Code-Block-2 which depends on Code-Block-1, and the variables in Code-Block-1 have been sanitized and validated, then do I need to re-sanitize and re-validate the variables from Code-Block-1 in Code-Block-3?

    Here is an example...

    Code-Block-1:
    PHP Code:
        // Check for Photo in DB. (3)
        
    if (!empty($currPhotoName)){
            
    // Photo Exists in DB.
            
    $currPhotoNameExists TRUE;
        }

        
    // Check for Photo-File on Server. (4)
        
    if (is_file(WEB_ROOT 'uploads/' $currPhotoName)){
            
    // Photo-File Exists.
            
    $currPhotoFileExists TRUE;    
        } 

    Code-Block-2:
    PHP Code:
            // Get RealPath of Current-Photo. (6)
            
    if (($currPhotoNameExists == TRUE) && ($currPhotoFileExists == TRUE)){
                
    // Photo exists in DB.
                // Photo-File exists on Server.
                //
                // Return canonicalized absolute pathname.
                // (**NOTE: If file cannot be found, realpath() will return FALSE.)
                
    $currRealPath realpath("../uploads/$currPhotoName");
                
            }else{
                
    // Throw Error... 

    Code-Block-3:
    PHP Code:
                // Delete Current-Photo (8a).
                
    if (($currPhotoNameExists == TRUE) && ($currPhotoFileExists == TRUE) && ($currRealPath != FALSE)){
                    
    // Valid Values.

                    // Unlink Photo-File. (8a1)
                    
    unlink($currRealPath); 

    Do I really need to have those 3 checks in the IF statement in this final code-block??

    After-all, I already checked earlier to ensure that there is a $currPhotoName and that a file exists for $currPhotoName and that a valid RealPath was returned.


    The logical part of me says, "Just unlink the file and be done with it".

    The emotional - read "paranoid" - part of me says, "You better be doubly sure that you have a valid RealPath or else the unlink function will throw an Warning and you'll have a mess!!!"


    I really, really, really try and be thorough when I code, but I am wondering if I have crossed the line into just plain paranoia, and thus I'm not being an efficient coder...

    What do you gurus think??

    Sincerely,


    Debbie

  2. #2
    Patience... bronze trophy solidcodes's Avatar
    Join Date
    Jul 2006
    Location
    Philippines
    Posts
    936
    Mentioned
    3 Post(s)
    Tagged
    0 Thread(s)
    I think one checking is enough.
    Also you have redundant comments there.
    Quality codes are optimized and tested...
    Click here for inspiration..

  3. #3
    SitePoint Addict bronze trophy vectorialpx's Avatar
    Join Date
    Dec 2012
    Location
    Bucharest
    Posts
    247
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    One good checking is enough.
    About files, if your files come from external sources, you must validate your request.
    Example:

    PHP Code:
    <?php

    $file2check 
    $_REQUEST['myfile'];

    // ---

    // rule no. 1: NEVER DO THIS
    if( is_file(WEB_ROOT 'uploads/' $file2check) ) {
        
    downloadTheFile$file2check );
    }
    /* The request may be '../config.php'
    (where you have your database connection) and is_file will return true */

    // ---

    // personal choice: I only keep on my server alpha-num names.
    // don't have on your server files like "the ni$e car.jpg" - it's a bad practice; use only "programmer" names -> "the_nice_car.jpg"
    // Advice of best check: allow only files that are on the server AND that are into a whitelist (into your database or into some "allowed-files" array)
    // example:

    if(
        
    checkIfFileExistsIntoYourDatabase$file2check ) && // it's a must!
        
    is_file(WEB_ROOT 'uploads/' $file2check)
    ) {
        
    downloadTheFile$file2check );
    }

    // ---

    // if you don't have a whitelist, make sure the filename is filtered
    // allow only alphanum and some chars (_, -, . and ,) into your names
    $namewithoutallowedchars str_replace(array('_''-'',''.'), ''$file2check);
    if( 
    false == ctype_alnum($namewithoutallowedchars) ) {
       die(
    'The name has bad chars!');
    }
    Be nice to nerds. Chances are you'll end up working for one - Bill Gates
    > photos | admin panel


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •