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:


	// 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:


		// 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:


			// 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

I think one checking is enough.
Also you have redundant comments there.

One good checking is enough.
About files, if your files come from external sources, you must validate your request.
Example:

<?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!');
}