Page 1 of 2

making a web app - is my core file secure?

Posted: Sat Jun 30, 2007 4:02 pm
by itsalmostreal
feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]


im working on a web app, and i wanted to get some feedback on a script that is run whenever any page is parsed ( ie its just required at the beginning of any php script)...

if the below code were saved as 'core.php' in a folder outside the html doc root along with all required pages within this script, then every page can start as...

Code: Select all

<?php
require "../../core/core.php"

- page contents

?>
anyhow, i just wanted to get some feedback on the code, and hopefully uncover any problems that might exist.

Code: Select all

<?php	

// process time start; get page process begin time, assign to variable $process_start
	$mtime = microtime(); 
	$mtime = explode (" ", $mtime); 
	$mtime = $mtime[1] + $mtime[0]; 
	$process_start = $mtime;


// root directory
	$root_dir = 'http://localhost/html/';


// database connection file
	require "mysql_connect.php";


// session handling
	session_start();

	if (!isset($_SESSION['username']) || !isset($_SESSION['password'])) {
		$logged_in = 0;

	} else {

		if(!get_magic_quotes_gpc()) {
			$_SESSION['username'] = addslashes($_SESSION['username']);
		}

		$pass = mysql_query("SELECT `password` FROM `users` WHERE `username` = '".$_SESSION['username']."'");

		if(mysql_num_rows($pass) != 1) {
			$logged_in = 0;
			unset($_SESSION['username']);
			unset($_SESSION['password']);
		}

		$db_pass = mysql_fetch_assoc($pass);

		$db_pass['password'] = stripslashes($db_pass['password']);
		$_SESSION['password'] = stripslashes($_SESSION['password']);

		if($_SESSION['password'] == $db_pass['password']) { 

			$logged_in = 1;

			$user_info = mysql_query("SELECT * FROM `users` WHERE `username` = '".$_SESSION['username']."'");
			$db_user_info = mysql_fetch_assoc($user_info);	// load user info into array $db_user_info	
			unset($db_pass['password']);

		} else {
			$logged_in = 0;
			unset($_SESSION['username']);
			unset($_SESSION['password']);
		}
	}

	$_SESSION['username'] = stripslashes($_SESSION['username']);
	
	
// date and time variables
//	- use these variables on any page to get date/time information
//		- '$the_date' - day of the week, month number of day, year; ex - Friday, April 20th, 2007
//		- '$epoch_stamp' - epoch timestamp (seconds since the unix epoch)
//		- '$epoch_string' - general string of time and date information; ex - 12:00 on 12/03/1982	 
	$the_date = date('l, F jS, Y');
	$epoch_stamp = date("U");
	$epoch_string = strftime('%I:%M on %m/%d/%y ', $epoch_stamp);


//	- make_safe($variable);
//	- trims and encodes html characters for safe handling
	function make_safe($variable) {
		if (get_magic_quotes_gpc) {
			$variable = stripslashes($variable);
			$variable = trim($variable);
			$variable = htmlentities($variable, ENT_QUOTES);
		} else {
			$variable = trim($variable);
			$variable = htmlentities($variable, ENT_QUOTES);
			$variable = addslashes($variable);
		}
		return $variable;
	}

// perform security functions on POST and GET data
//	- perform functions and save as shortnames: $p for POST and $g for GET
	if ($_POST) {
		if (is_array($_POST)) {
			$_POST = array_map("make_safe", $_POST);
		} else {
			$_POST = make_safe($_POST);
		}
		$p = $_POST; // rename the superglobal $_POST to $p
	}

	if ($_GET) {
		if (is_array($_GET)) {
			$_GET = array_map("make_safe", $_GET);
		} else {
			$_GET = make_safe($_GET);
		}
		$g = $_GET; // rename the superglobal $_GET to $g
	}
	
?>

feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]

Posted: Sat Jun 30, 2007 4:06 pm
by Benjamin
Your going to run into issues with $_GET or $_POST values that are arrays.

For example, what happens if $_POST['foo'] is an array?

Posted: Sat Jun 30, 2007 4:34 pm
by itsalmostreal
i never planned on using foo as an array key. why would i want to do that?

Posted: Sat Jun 30, 2007 4:41 pm
by John Cartwright
There are many, many reasons. For instance, a group of checkboxes?

Posted: Sat Jun 30, 2007 5:03 pm
by itsalmostreal
thats not a limiting factor; you can assign anything to a checkbox or select field. i usually use either NULL or 0. if you force yourself to use foo i'd say that is limiting yourself...

but anyhow, do you see any other issues? and thanks for the feedback btw :)

Posted: Sat Jun 30, 2007 5:11 pm
by John Cartwright
I'm not talking about the value, but the name element

Code: Select all

<?php
   if (isset($_POST['somename'])) {
      echo '<pre>';
      print_r($_POST['somename']);
   }
?>

<input type="checkbox" name="somename[]" value="foo">
<input type="checkbox" name="somename[]" value="bar">

Posted: Sat Jun 30, 2007 5:21 pm
by superdezign
make_safe() doesn't make_sense. If magic quotes are on, you get rid of slashes, otherwise.. you add them? Why?

Also, you shouldn't need to run stripslashes on a password because it should be encrypted. Where's your encryption?

Posted: Sat Jun 30, 2007 5:32 pm
by itsalmostreal
i still dont see how foo is getting in there. when i generate a list set of check boxes, i use foreach and a counter make up a name for the entire set, then append the count to the name.

function form_checkboxes($name, $values) {
foreach ($values as $value) {
print '<input type="checkbox" name="'.$name.'_'.$value.'">'.$value.'<br>';
}
}

then to see if an item has been check is as easy as...

foreach ($values as $value) {
if ($p[''.$name.'_'.$value.''] == "on") {
print $value.' was checked!<BR>';
}
}

am i missing something here? this example uses an array, and i've adapted it so that you can get a list of values from a db, and then have a list of checkboxes for em all. seems like the way to do it. still not sure why you guys are using foo, or where its coming from.

Posted: Sat Jun 30, 2007 5:39 pm
by superdezign
'foo' is just a term used for examples. It means absolutely nothing.

Posted: Sat Jun 30, 2007 5:39 pm
by itsalmostreal
okay, that was a stupid mistake. heres the new make_safe:

Code: Select all

function make_safe($variable) {
		if (get_magic_quotes_gpc) {
			$variable = stripslashes($variable);
			$variable = trim($variable);
			$variable = htmlentities($variable, ENT_QUOTES);
		} else {
			$variable = trim($variable);
			$variable = htmlentities($variable, ENT_QUOTES);
		}
		return $variable;
	}
not sure why i was adding slashes when gpc was off; i've written a bunch of mysql functions that add slashes to varibles being submitted to the db. if i had turned magic quotes off i guess i would have seen this.

as far as the passwords are conerned, i think youre right again. no need to strip slashes. and my passwords are stored as an md5 hash. do i need some sort of encryption there? i dont think so, but then again, im making mistakes here so...

thanks!

Posted: Sat Jun 30, 2007 5:41 pm
by Benjamin
Let me ask it this way..

What happens if you pass your make_safe function an array?

Posted: Sat Jun 30, 2007 5:45 pm
by itsalmostreal
wait, do i need to strip slashes from the session password? im not to clear on that...

Posted: Sat Jun 30, 2007 5:49 pm
by itsalmostreal
well both POST and GET values are covered there. they go thru the process in the next section of code. say if POST is an array, i use the array_walk function to perfom the make_safe function on all values. the same for GET data.

are you saying i should build the if 'is_array' statement into the make_safe function instead?

Posted: Sat Jun 30, 2007 6:01 pm
by superdezign
itsalmostreal wrote:wait, do i need to strip slashes from the session password? im not to clear on that...
You need to encrypt your passwords. Unencrypted data is an unnecessary risk.

Posted: Sat Jun 30, 2007 6:03 pm
by Benjamin
itsalmostreal wrote:i use the array_walk function to perfom the make_safe function on all values
What if one of the values IS an array?