multiple action in a single page

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

Post Reply
shivam0101
Forum Contributor
Posts: 197
Joined: Sat Jun 09, 2007 12:09 am

multiple action in a single page

Post by shivam0101 »

I saw an example of writing safer code in a book. Can anyone tell me is it really good and safe design. Is it safe to use

Code: Select all

$_REQUEST['cmd']?
Here is the example,


Code: Select all

switch($_REQUEST['cmd']) {
case 'updateNews': updateNews(); break;
case 'insertNews': insertNews(); break;
case 'editArticle': editArticle(); break;
case 'newArticle': newArticle(); break;
default: die("No such command: ".$_REQUEST['cmd']);
}


function insertNews() {
$sql = mysql_real_escape_string(
"INSERT INTO News ".
"(headline,text) ".
"VALUES ('".$_POST['headline']."','"
Command Purpose
editArticle Show the form initialized with an existing article and ready to edit.
newArticle Show the empty form ready to enter a new article.
insertNews Run INSERT to save a new article.
updateNews Run UPDATE to save changes to an existing article.
Listing 15.2 News form example refactored to use command functions
KEEPING THE IMPLEMENTATION SIMPLE 353
.$_POST['text']."') "
);


mysql_query($sql);
header("Location: http://localhost/newslist.php");
exit;
}


function updateNews() {
$sql = mysql_real_escape_string(
"UPDATE News SET ".
"headline = '".$_POST['headline']."',".
"text = '".$_POST['text']."' ".
"WHERE id = ".$_POST['id']
);


mysql_query($sql);
header("Location: http://localhost/newslist.php");
exit;
}


function editArticle() {
$sql = mysql_real_escape_string(
'SELECT id,text,headline '.
'FROM News WHERE id = '.$_REQUEST['id']
);


$r = mysql_query($sql);
$article = mysql_fetch_assoc($r);
showForm('updateNews',$article);
}


function newArticle() {
showForm('insertNews',array());
}


function showForm($command,$article) {
?>
<html>
<body>
<h1>Submit news</h1>
<form method="POST">
<input type="hidden" name="cmd"
value="<?php echo $command ?>">
<input type="hidden" name="id"
value="<?php echo $article['id'] ?>">
Headline:
<input type="text" name="headline"
value="<?php echo $article['headline'] ?>"><br>
Text:
<textarea name="text"ols="50" rows="20">
<?php echo $article['text'] ?></textarea><br>
<input type="submit" value="Submit news">
</form>
</body>
</html>
<?php
}
?>
Last edited by shivam0101 on Fri Sep 21, 2007 7:47 pm, edited 1 time in total.
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

Whats your question?
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

$_REQUEST is a conglomeration of several super globals. Use the proper super globals if possible. Otherwise create your own conglomeration. $_REQUEST is controlled by a initialization file setting, which may vary from server to server. As long as you're aware of the variance and willing the accept that it may not contain the data you are looking for, or may contain the data via alternate means.. it's just as dangerous at $_GET and $_POST, with the aforementioned caveats.
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

I would also read the manual about mysql_real_escape_string
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

The concept in itself is fine. No problems there.

However a couple parts (mentioned above) in the actual script are a cause for concern.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Is this a correct use of mysql_real_escape_string?

Code: Select all

function updateNews() {
$sql = mysql_real_escape_string(
"UPDATE News SET ".
"headline = '".$_POST['headline']."',".
"text = '".$_POST['text']."' ".
"WHERE id = ".$_POST['id']
);
I've never seen it being used like that.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Nope. The data only needs escaping. ;)
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Post by Mordred »

The book needs recylcing ;)
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

Mordred wrote:The book needs recylcing ;)
Or maybe DevNet should have a book. :P
Post Reply