Well in any case, it's becoming much better. ;) It's not the most normal forum, but it's becoming more promising anyway. I still think MD5ing everything is redundant, only the password should really be hidden from the eyes. The reasoning why it's done differently is a little better defined, but I still think it needs a little work. I can't pin-point exactly what it is yet, but something seems incomplete. Good work so far, keep moving forward adding more functions like edit/delete. As you're doing that, keep in mind Administration, and most importantly security.
You can use a simple where clause for the posts/threads:
Code:
/* Update Query.. */ WHERE `rank_id` = "'.$GLOBALS['rank_id'].'" OR `user_id` = "'.$GLOBALS['user_id'].'"
The key, is to be absolutely confident and sure that your variables cannot get redefined by a malicious user. That is the one sure key to being secure. If you can keep an array of safe, secure variables extracted directly through a confirmed database query, you can do allot of very cool things. [Nothing gets passed, 'SELECT id, rank_id FROM users WHERE user="user" AND pass="pass"', for instance.. You must also be sure they cannot be redefined later in the script]. You can use your own methods, though simpler procedures are much easier to keep track of on allot of levels.
Things that get loaded on every page must be as simple as possible, for that's the very heart of the engine, that can slow down every page. For example, using SESSION variables allows a 'first-time-only' load on many variables. Site settings, global theme properties, and even user-data can be loaded one time from the database, only on the first page-load. Could be done with a single line of code; If the sessions exist, then they don't get loaded from the database. Logically speaking,
PHP Code:
if(!isset($_SESSION['site_settings']))
{
$site_settings = new site_load_settings();
$site_settings->create_session('_site'); //always use a prefix for things like this
}
If you're perfectly confident the session will be what it needs to be, this saves allot of execution time.
Can you tell me what's wrong with this script?
PHP Code:
<?php
$post_id = mysql_real_escape_string($_POST['post_id']);
$post_title = mysql_real_escape_string($_POST['post_title']);
$post_body_HTML = BBcodeToHtml($_POST['post_body']);
$post_body_BB = $_POST['post_body'];
mysql_query('UPDATE `posts` SET `title` = "'.$post_title.'",
`body_HTML` = "'.$post_body_HTML.'",
`body_BB` = "'.$post_body_BB.'"
WHERE `id` = "'.$post_id.'"') or $errMsg = mysql_error();
if(strlen($errMsg)<1)
$goodMsg = 'Post Updated!';
?>
<form method="post" action="">
<input type="hidden" name="post_id" value="3" />
<input type="text" name="post_title" value="The Original Title" />
<br />
<textarea name="post_body">[b]Original Post Body with [u]BBcode[/u][/b]
...
</textarea><br />
<input type="submit" name="edit_post" value="Update Post" />
</form>
If you can find all the security risks in there, and you take in what I said above, you should be fairly confident with security.
So, see anything wrong in the code above?
Reply With Quote
Originally Posted by s-p-n
