making a web app - is my core file secure?

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

itsalmostreal
Forum Newbie
Posts: 8
Joined: Sat Jun 30, 2007 3:50 pm

making a web app - is my core file secure?

Post 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]
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post 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?
itsalmostreal
Forum Newbie
Posts: 8
Joined: Sat Jun 30, 2007 3:50 pm

Post by itsalmostreal »

i never planned on using foo as an array key. why would i want to do that?
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

There are many, many reasons. For instance, a group of checkboxes?
itsalmostreal
Forum Newbie
Posts: 8
Joined: Sat Jun 30, 2007 3:50 pm

Post 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 :)
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post 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">
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post 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?
itsalmostreal
Forum Newbie
Posts: 8
Joined: Sat Jun 30, 2007 3:50 pm

Post 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.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

'foo' is just a term used for examples. It means absolutely nothing.
itsalmostreal
Forum Newbie
Posts: 8
Joined: Sat Jun 30, 2007 3:50 pm

Post 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!
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post by Benjamin »

Let me ask it this way..

What happens if you pass your make_safe function an array?
itsalmostreal
Forum Newbie
Posts: 8
Joined: Sat Jun 30, 2007 3:50 pm

Post by itsalmostreal »

wait, do i need to strip slashes from the session password? im not to clear on that...
itsalmostreal
Forum Newbie
Posts: 8
Joined: Sat Jun 30, 2007 3:50 pm

Post 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?
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post 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.
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post 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?
Post Reply