break it?

Sic

Sic

Sic

Soldato
Joined
9 Nov 2004
Posts
15,365
Location
SO16
hey guys, i've just finished writing the comments section on my blog, and i was wondering if any of you security experts would like to have a go at spamming it, or putting things in that shouldn't be there or any of those other naughty tricks that can be done?
i'm not doing any checks for email address format or anything like that because it's using email for verification, so it'll be deleted after 5 days if it's not verified anyway.

also, if you can have a go at the login script, that'd be swell (i know that error.php doesn't exist ;))

it's at www.sameagain.net

also, if there's any anomalies with the xhtml/css, i haven't finished checking all that yet, so cut me some slack ;)

cheers
 
I'd assume SQL insertion is the primary concern rather than the likes of XSS.

If so, you're probably better off doing a code review of all the places user input can be processed rather than relying on people throwing random data at it.
 
yeah, i've looked at all the places stuff could be put in, but i wasn't sure if there was some magic tricks that i've missed out on. i've written a function that cleans input...so everything a user puts in goes through that, but like i said...i was just wondering.

Code: 
class cComments extends cDb
	{
		var $post_id;
		var $commentNo;
		
			function calcComments($query)
			{
			cDb::buildArray($query);
			
				if ($this->arrayCount == 0)
				{
					$this->commentNo = "No comments";
				}
				else if ($this->arrayCount == 1)
				{
					$this->commentNo = "1 comment";
				}
				else
				{
					$this->commentNo = $this->arrayCount." comments";			
				}
			}	
			
			function echoComments()
			{	
				$this->post_id = intval($_REQUEST['pid']);
				cComments::calcComments("SELECT * FROM comment WHERE post_id = $this->post_id AND status = 2 ORDER BY date DESC");
				
				echo "<div class='blog'>";
				echo "<div class='commentForm'>";
				
				echo "<h1>".$this->commentNo."</h1></div>";
				
				cComments::writeComments();
				cComments::commentForm();
				
				echo "</div>";			
		}
	
		function commentForm()
		{
			$this->post_id = intval($_REQUEST['pid']);
			
			echo "<div id='posted'></div>";
			echo "<div id='commentForm'>";
			echo "<h1>Leave a reply</h1>";
			echo "<form name='commentForm'>";
			echo "<input type='hidden' value='".$this->post_id."' name='pid' style='border: none;' />";
			echo "<span class='label'>Name:</span><span id='errorName'></span>";
			echo "<input type='text' name='name' />";
			echo "<span class='label'>Email: (required)</span><span id='errorEmail'></span>";
			echo "<input type='text' name='email' />";
			echo "<span class='label'>URI: http://</span>";
			echo "<input type='text' name='url' />";
			echo "<span class='label'>Comment:</span><span id='errorComment'></span>";
			echo "<textarea name='comment' rows='10' cols='22'></textarea>";
			echo "<input type='button' value='holler!' onClick='checkCommentForm()' name='loadingChange' />";
			echo "</form>";
			echo "</div>";
		}
		
		function makeComment()
		{
			$post_id = cDb::cleanInput("$_REQUEST[pid]");
			$name = cDb::cleanInput("$_REQUEST[name]");
			$email = cDb::cleanInput("$_REQUEST[email]");
			$url = cDb::cleanInput("$_REQUEST[url]");
			$body = cDb::cleanInput("$_REQUEST[comment]");
			$referrer = $_SERVER['HTTP_REFERER'];
			
			if ($referrer == "http://www.sameagain.net/?pid=$post_id")
			{
				if ($name == "" || $email == "" || $body == "")
				{
					echo "It appears that you've somehow arrived without the aid of JavaScript. Please enable it to continue";
				}
				else
				{
					cDb::randomString(20);
					$code = $this->randomString;
					@cDb::buildArray("SELECT * FROM comment WHERE code = $code");
					if ($this->arrayCount > 1)
					{
						unset($code);
						unset($this->randomString);
						cDb::randomString(20);
						$code = $this->randomString;
					}
					
					mysql_query("INSERT INTO comment (post_id,date,author,email_address,body,status,code,url) VALUES('$post_id',NOW(),'$name','$email','$body',0,'$code','$url')");
					
					cComments::mailCode("$code", "$email", "$name");
					
					echo "$name, check your email for a link to verify your comment. Thanks";
					
				}
			}
			else
			{
				echo "This blog doesn't support remote comment posting. Sorry";
			}			
		}
		
		function writeComments()
		{
			foreach ($this->blog as $comment)
					{
						echo "<div class='comment'>";
						if ($comment['url'] !== "")
							{
								echo "<a href='http://".$comment['url']."'>".$comment['author']."</a> says:<br />";
							}
						else
							{
								echo "".$comment['author']." says:<br />";
							}
							
							$comment['date'] = date('l jS F Y' , strtotime($comment['date']));
							
								echo "<span class='date'>".$comment['date']."</span>";
								echo $comment['body'];
							
						echo "</div>";
					}
		}
		
		function checkCode()
		{
			$code = cDb::cleanInput("$_REQUEST[code]");
			
			cDb::buildArray("SELECT * FROM comment WHERE code = '$code'");
			
			$pid = $this->blog[0]['post_id'];
		
			if ($this->numrows == 1)
			{
				$update = mysql_query("UPDATE comment SET status='1' WHERE code = '$code'");
				header("Location: /?pid=$pid");
			}
			else
			{
				echo "Your comment was not found. Please click the link from your email to verify the comment";
			}		
		}
		
		function mailCode($code, $email, $name)
		{
				$url = "http://www.sameagain.net/checkCode.php?code=$code";
				$subject = "Verify your comment at SameAgainDotNet";
				$body = "<html>
									<head></head>
									<body>Hi<br />
									Before your comment can appear on SameAgainDotNet, you need to verify it by clicking/visiting this link:<br />
									<a href='$url'>clicky!</a><br />
									or copy/paste this:<br /><br />
									$url<br />
									<br />
									Jasper
									</body>
									</html>";
				$headers  = 'MIME-Version: 1.0' . "\r\n";
				$headers .= 'Content-type: text/html; charset=iso-8859-1' . "\r\n";
				$headers .= 'From: SameAgainDotNet <[email protected]>' . "\r\n";
				
				mail($email,$subject,$body,$headers);
		}
	}
and the user input cleaner is here:
Code: 
			function cleanInput($value)
			{
				 $value = trim($value);
				 if(get_magic_quotes_gpc())
				 {
						$value = stripslashes($value);
				 }
				 if(!is_numeric($value))
				 {
						$text = @mysql_real_escape_string($value);
						if($text === FALSE)
						{
							 $text = mysql_escape_string($value);
						}
						$value = "$text";
				 }
				 return($value);
			}
if you can see anything in there that i've omitted, i'd love to hear it :)

if i've missed it, it's because i'm not aware that it should be done...so explanations in layman's terms if you're kind enough to respond!

thanks
 
With regards to the cleaning function, it might be a good idea to make it clean arrays recursively. Just call is_array() on the input, and if it returns true, recurse on it. This'll avoid problems with POST arrays and the like (if you're calling the function on $_GET, $_POST, and $_COOKIE on each request, rather than on each input as it's used) :)
 
A note (again) that asking people to hack your site can lead to legal problems for the perpurtraitors (spelling?) regardless of having permission to do so, or not.
 
Inquisitor said:
With regards to the cleaning function, it might be a good idea to make it clean arrays recursively. Just call is_array() on the input, and if it returns true, recurse on it. This'll avoid problems with POST arrays and the like (if you're calling the function on $_GET, $_POST, and $_COOKIE on each request, rather than on each input as it's used) :)
Click to expand...

i kinda understand what you're saying there - could you give me a simple example of how it'd be done, as i'm not really sure what you'd do :)
Dj_Jestar said:
A note (again) that asking people to hack your site can lead to legal problems for the perpurtraitors (spelling?) regardless of having permission to do so, or not.
Click to expand...
fair point. don't try and hack my site - but if you can see flaws in the code that would lead to it, i'd appreciate you letting me know :)
 
Sic said:
i kinda understand what you're saying there - could you give me a simple example of how it'd be done, as i'm not really sure what you'd do :)
Click to expand...
Well, this is more for removing those pesky slashes from Magic-Quotes-mutilated superglobal variables, so it should be used in conjunction with what you've already got:
Code: 
function RemoveSlashes($input)
{
	if (!get_magic_quotes_gpc())
	{
		return $input;
	}
	
	if (is_array($input))
	{
		foreach ($input as &$value)
		{
			$value = RemoveSlashes($value);
		}
		
		return $input;
	}
	else
	{
		return stripslashes($input);
	}	
}

For example, you could call this at the beginning of each request:
Code: 
$_GET = RemoveSlashes($_GET);
$_COOKIE = RemoveSlashes($_COOKIE);
$_POST = RemoveSlashes($_POST);
This means that you no longer have to worry about stripping slashes from any input. You do, of course, however, have to escape input before using it in queries, etc. :)
 
ah right, i get you. didn't realise you could use a function within a function like that! everyday's a school day!

thanks for your help :)
 
not really what you were asking for but i love the colours and the logo, it looks mint.
icon14.gif
 
You seem to be sorted with the security, so general comments. Opera is giving a horizontal scroll bar for the column, at the bottom of the page, which is wierd. Also, I see you incorporated the repeating background into the header. I'm surprised you couldn't get it looking un-jaggy with a transparent background.

Nice work :)
 
You must log in or register to reply here.
Back
Top Bottom