Edit Information No Longer Work????

taurus5_6

I have a project which has been working quite well for a long time. Suddenly, the "admin" page for that site I made no longer work? It has a portion where the website admin (who is not a web developer or know nothing about php or mysql) can change information, upload picture, etc on an interface I created specially for him.

Problem is, it is no longer functioning lately. All the changes made does not change anything in the database anymore? what could possibly be wrong? attached are the files involved:

LIST PAGE

<?php
include("globalfn.php");
// decalres $intdelete, $ann_id
$intdelete = $_GET["intdelete"]; //get action variable from link
$ann_id = $_GET["ann_id"]; //get action variable from link

//Check if logged-in
//$adminusername = $_COOKIE['adminusername'];
//$adminpassword = $_COOKIE['adminpassword'];
//if(trim(@$adminusername)=="" and trim(@$adminpassword)=="") {
//	header("Location: login.php");
//}

//Delete
if(@$intdelete == 1) {
	$sql = "DELETE FROM tbl_announce WHERE ann_id=$ann_id";
	put_result($sql);
}
?>
<html>
<head>
<title>admin</title>
<link rel="stylesheet" type="text/css" href="admin.css">
<script language="JavaScript" type="text/javascript">
<!--
function areyousure() {
	ask = confirm("Are you sure you want to delete this entry?");
	if(!ask) {	return false;	}
}
//-->
</script>
</head>
<body>
<table border="0" cellspacing="0" cellpadding="0" width="100%">
	<?php include("header_admin.php"); ?>
	<tr><td><b>List of Announcements</b></td></tr>
	<tr><td><a href="home.php">[home]</a> | <a href="ann_add.php">[add announcements]</a></td></tr>
	<tr><Td>&nbsp;</Td></tr>
	<tr><td><table border="1" cellpadding="2" cellspacing="2" width="100%">
	<tr>
		<td valign="top"><b>Title</b></td>
		<td valign="top"><b>Date Created</b></td>
		<td valign="top"><b>Status</b></td>
		<td valign="top" align="center" width="50">&nbsp;</td>
		<td valign="top" align="center" width="50">&nbsp;</td>
	</tr>
	<?php $sql = "SELECT title, ann_id, status FROM tbl_announce";
	$result = put_result($sql);
	while($row = mysql_fetch_object($result)) { ?>
		<tr onMouseOut="this.bgColor = ''" onMouseOver="this.bgColor = '#CCCCCC';">
			<td valign="top"><?php echo($row->title); ?></td>
			<td valign="top"><?php echo(date("M-d-Y", strtotime($row->dt_created))); ?></td>
			<td valign="top">
			<?php 
			//if ($row->status == 1) { echo("Online"); } else { echo("Offline"); }
			//thanks bradgrafelman
			echo ($row->status == 1 ? "Online" : "Offline");  ?>
			</td>
			<td valign="top" align="center" width="50"><a href="ann_edit.php?ann_id=<?php echo($row->ann_id); ?>" class="textgoldsmall">edit</a></td>
			<td valign="top" align="center" width="50"><a href="ann_list.php?ann_id=<?php echo($row->ann_id); ?>&intdelete=1" onClick="return areyousure()">delete</a></td>
		</tr>
	<?php } ?>
	</table></td></tr>
</table>
</body>
</html>

EDIT PAGE:

<?php
include("globalfn.php");
$ann_id = (isset($_GET['ann_id']) ? $_GET['ann_id'] : ''); 

//Check if logged-in
//$adminusername = $_COOKIE['adminusername'];
//$adminpassword = $_COOKIE['adminpassword'];
//if(trim(@$adminusername)=="" and trim(@$adminpassword)=="") {
//	header("Location: login.php");
//}

if($REQUEST_METHOD == "POST") {
	$currdate = date("Y-m-d");
	$content = repl_enter($content);
//	$sql = "UPDATE tbl_announce SET title = '$title', content = '$content', dt_created = '$currdate', status = $status WHERE ann_id = $ann_id";
// 	thanks bradgrafelman
	$sql = sprintf("UPDATE tbl_announce SET title='%s', content='%s'," 
             . "dt_created=CURDATE(), status=%d WHERE ann_id=%d", 
             mysql_real_escape_string($_POST['title']), 
             mysql_real_escape_string($_POST['content']), 
             $_POST['status'], 
             $ann_id 
             );
	put_result($sql);
	header("Location: ann_list.php");
}

//Select Incentive
$sql = "SELECT title, ann_id, content, status FROM tbl_announce WHERE ann_id = '$ann_id'";
$result = put_result($sql);
$row = mysql_fetch_object($result);
?>
<html>
<head>
<title>admin</title>
<link rel="stylesheet" type="text/css" href="admin.css">
<script language="JavaScript" type="text/javascript">
<!--
function cmdSubmit() {
	if (document.frm.title.value == "") {
		alert("Please key in Title");
		document.frm.title.focus();
		document.frm.title.select();
		return false;
	}
	if (document.frm.content.value == "") {
		alert("Please key in Content");
		document.frm.content.focus();
		document.frm.content.select();
		return false;
	}
	document.frm.submit();
}
//-->
</script>
</head>
<body>
<form action="ann_edit.php" name="frm" METHOD="POST">
<table border="0" cellspacing="0" cellpadding="0">
	<?php include("header_admin.php"); ?>
	<tr><td colspan="2"><b>Edit Announcement</b></td></tr>
	<tr><td colspan="2"><a href="home.php">[home]</a> | <a href="ann_list.php">[back]</a></td></tr>
	<tr><td>&nbsp;</td></tr>
	<tr>
		<td>Title: &nbsp; </td>
		<td><input type="text" name="title" size="60" value="<?php echo($row->title); ?>" maxlength="200"></td>
	</tr>
	<tr>
		<td valign="top">Content: &nbsp; </td>
		<td><textarea name="content" rows="20" cols="50"><?php echo(repl_br($row->content)); ?></textarea></td>
	</tr>
	<tr>
		<td>Status: &nbsp; </td>
		<td>
			<input type="radio" name="status" value="1" <?php if ($row->status == 1) { echo("checked"); } ?>> Online &nbsp; &nbsp; 
			<input type="radio" name="status" value="2" <?php if ($row->status == 2) { echo("checked"); } ?>> Offline
		</td>
	</tr>
	<tr>
		<td>&nbsp;</td>
		<td><br><input type="button" name="btnsave" value="Submit" onClick="cmdSubmit(); return false;"></td>
	</tr>
	<input type="hidden" name="ann_id" value="<?php echo($ann_id) ?>">
</table>
</form>
</body>
</html>

Please help!!!
:mad:

bradgrafelman

Sounds like register_globals got turned off, which it should have been in the first place. Here's several changes I would suggest:

First Script:

Where is $intdlete ever defined? If this is supposed to come from the URL, use $GET['intdelete'] instead. If it's being POSTed as from a form, use the $POST superglobal instead.
Where is $ann_id ever defined? Same deal as above.
Don't use short tags. Ever. Use the full "<?php" tag.
Try to avoid doing a "SELECT " query - only select the columns you're going to use. Even if you currently use all columns, it's best to name them (for scalability reasons).
You're using 2 functions in PHP to convert the date into a format you want to display it in, though MySQL only needs to use one (assuming dt_created is a DATE column, which it looks like it should be): DATE_FORMAT().
Just to introduce you to some new syntax, the ternary operator would require a bit less typing to display the status:
```
echo ($row->status == 1 ? "Online" : "Offline");
```
For more information, see this manual page: Ternary Operator.

And for the edit page:

To avoid errors, it's better to assign variables taken from the $GET or $POST arrays like so:
```
$ann_id = (isset($_GET['ann_id']) ? $_GET['ann_id'] : '');
```
Again, $REQUEST_METHOD isn't defined. Try using the $SERVER array this time. If register_globals is truly turned off, this is the biggest problem you need to fix in this script, though you should definitely address the rest.
Funny that you named the variable to hold the current date "currdate", because this is almost the name of the MySQL function you could use instead: CURDATE[/man]().
All of the variables in your SQL query are, I'm assuming, coming from the POSTed data, though you don't define them. Again, use the $_POST superglobal array.
It appears as though you're vulnerable to SQL injection attacks. User-supplied data should never be placed directly into a SQL query! Instead, sanitize it with a function such as [man]mysql_real_escape_string[/man](). Here's how I like to do it, using [man]sprintf[/man]():
```
$sql = sprintf("UPDATE tbl_announce SET title='%s', content='%s',"
             . "dt_created=CURDATE(), status=%d WHERE ann_id=%d",
             mysql_real_escape_string($_POST['title']),
             mysql_real_escape_string($_POST['content']),
             $_POST['status'],
             $ann_id
             );
```
Using sprintf() is just a preference, but it serves an additional purpose other than just making it look cleaner: using %d, I know that the last two variables can't contain quotes or things that would be used to do SQL injection attacks. Thus, there's no need to use mysql_real_escape_string(). If you don't use sprintf(), however, you lose this protect. In that case, you'd want to make sure those are indeed integers and not something harmful; casting them to integers will do the job nicely: code=php$_POST['status'][/code]
When I'm UPDATE'ing a table where I know that there is only one unique row that I want to edit (e.g. there's only 1 row with the ann_id you're editing), I usually add a "LIMIT 1" onto the end of the query so that SQL knows it only should bother searching for the first row it comes to that matches the condition.
Same deal as above in regards to the "SELECT *" query. Also note that there's no need for the quotes around $ann_id since you're passing SQL a numeric value.
Also be aware that this query uses user-supplied data ($_GET["ann_id"]), so you should probably cast that value to an integer at the top of your script. That way, it can be considered "safe" for the rest of the script.

taurus5_6

Thanks a lot man... Seems like ive got lot of holes to fix here. For now, I will be going over your suggestions (well... tutorial :-) i guess) one by one. I will keep in touch. thanks

bradgrafelman

Woops.. I also forgot to post a manual link that talks about these "superglobal arrays" I referenced: [man]variables.predefined[/man]. Note that this link has a plethora of information regarding the new changes since the register_globals directive has been deprecated (for security reasons - also has info about this, too).

Don't let the size of the page or the number of links scare you, though; it all boils down to basically using one of the arrays instead of just a plain variable (ex. $_SERVER['PHP_SELF'] instead of $PHP_SELF).

laserlight

it all boils down to basically using one of the arrays instead of just a plain variable (ex. $_SERVER['PHP_SELF'] instead of $PHP_SELF).

To make it clear: it is a "plain variable" (in the sense that it is scalar), but now as an element of a superglobal array, thus differentiating it from a possible variable that you are more likely to define.

bradgrafelman

Heh, thanks laserlight - I couldn't find the right way to describe it so I gave up :p

laserlight

It just shows how long we have not been relying on register_globals, heheh. I too did not explain another thing: aside from preventing a potential conflict with variables that you might name, a possibly more important reason is to protect against variable poisoning, where the user could use the query string or a form (etc) to try and change the variables set in your script. By using $REQUEST, $GET, $POST, $COOKIE etc, you know what you are dealing with: incoming variables that need to be handled very carefully.

taurus5_6

Thanks people...

I tried doing some of the tips you gave. However, there's no luck till now. So I think I have to go through the step one of this form I made. ADDING AN ENTRY into my announcement section. Which also is not working anymore (it happened simultaneously).

ADD ENTRY PAGE

<?php
include("globalfn.php");
$ann_id = $_GET["ann_id"];

//Check if logged-in
//$adminusername = $_COOKIE['adminusername'];
//$adminpassword = $_COOKIE['adminpassword'];
//if(trim(@$adminusername)=="" and trim(@$adminpassword)=="") {
//	header("Location: login.php");
//}

if($REQUEST_METHOD == "POST") {
	$currdate = date("Y-m-d");
	$content = repl_enter($content);
	$sql = "INSERT INTO tbl_announce(title,content,dt_created,status) VALUES('$title','$content','$currdate',$status)";
	put_result($sql);
	header("Location: ann_list.php");
}
?>
<html>
<head>
<title>admin</title>
<link rel="stylesheet" type="text/css" href="admin.css">
<script language="JavaScript" type="text/javascript">
<!--
function cmdSubmit() {
	if (document.frm.title.value == "") {
		alert("Please key in Title");
		document.frm.title.focus();
		document.frm.title.select();
		return false;
	}
	if (document.frm.content.value == "") {
		alert("Please key in Content");
		document.frm.content.focus();
		document.frm.content.select();
		return false;
	}
	document.frm.submit();
}
//-->
</script>
</head>
<body>
<form action="ann_add.php" name="frm" METHOD="POST">
<table border="0" cellspacing="0" cellpadding="0">
	<?php include("header_admin.php"); ?>
	<tr><td colspan="2"><b>Add Announcement</b></td></tr>
	<tr><td colspan="2"><a href="home.php">[home]</a> | <a href="ann_list.php">[back]</a></td></tr>
	<tr><td>&nbsp;</td></tr>
	<tr>
		<td>Title: &nbsp; </td>
		<td><input type="text" name="title" size="60" maxlength="200"></td>
	</tr>
	<tr>
		<td valign="top">Content: &nbsp; </td>
		<td><textarea name="content" rows="20" cols="50"></textarea></td>
	</tr>
	<tr>
		<td>Status: &nbsp; </td>
		<td>
			<input type="radio" name="status" value="1" checked> Online &nbsp; &nbsp; 
			<input type="radio" name="status" value="2"> Offline
		</td>
	</tr>
	<tr>
		<td>&nbsp;</td>
		<td><br><input type="button" name="btnsave" value="Submit" onClick="cmdSubmit(); return false;"></td>
	</tr>
</table>
</form>
</body>
</html>