PHP Developers Network

A community of PHP developers offering assistance, advice, discussion, and friendship.
 
Loading
It is currently Wed Dec 12, 2018 10:44 am

All times are UTC - 5 hours




Post new topic Reply to topic  [ 7 posts ] 
Author Message
PostPosted: Tue Aug 21, 2007 1:01 pm 
Offline
Forum Contributor
User avatar

Joined: Thu Nov 10, 2005 1:31 pm
Posts: 355
Location: Broolyn, NY
This is a very simple forum for a small website. Does it look secure enough?

Syntax: [ Download ] [ Hide ]
<?php

        ob_start();

       

        /*

                Connect

        */


        require 'library/connect.php'; 

       

        /*

                Handle Form

        */




        if(isset($_POST['submit'])){

                $errors=array();

               

                //Check for empty fields

                if(!isset($_POST['name']) || empty($_POST['name'])){

                        $errors[]='Name';

                }

               

                if (!eregi('^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[_a-z0-9-]+(\.[_a-z0-9-]+)*(\.[a-z]{2,4})$',trim($_POST['email']))){

                        $errors[]='Email';

                }

               

                if(!isset($_POST['comment']) || empty($_POST['comment'])){

                        $errors[]='Comment';

                }

               

                //If all went ok

                if(empty($errors)){

                        $name=mysql_real_escape_string(trim($_POST['name']));

                        $email=mysql_real_escape_string(trim($_POST['email']));

                        $comment=mysql_real_escape_string(trim(nl2br($_POST['comment'])));



                        $query="INSERT INTO forum (name,email,comment,date) VALUES ('$name','$email','$comment',current_date)";

                        mysql_query($query) or die (mysql_error());

                        header('location:forum.php');

                } else {

                        $error="<p>There is either missing or innacurate data in the following field(s):</p>\n";

                        $error.="<ul>\n";

                        foreach($errors as $value){

                                $error.= "<li>$value</li>\n";

                        }

                        $error.="</ul>\n";

                        $error.="<p>Please re-enter your comment.</p>\n";

                }

        }



        /*

                Start Content

        */


        $pageTitle="Union County Comprehensive Plan - Forum";

        $banner='';

       

        $left='

        <p><strong>Comment</strong></p>

        <form method="post">

                Name<br />

                <input type="text" name="name" size="32" /><br />

                Email<br />

                <input type="text" name="email" size="32" /><br />

                Comment<br />

                <textarea name="comment" rows="5" cols="23"></textarea>

                <input type="submit" name="submit" value="Add Comment" />

        </form>

        '
;

       

        $content="<h3>Forum</h3>\n";   

               

        /*

                Display Comments

        */
     

        //pagination

        $rowsPerPage=5;

        $pageNum=1;

        if(isset($_GET['page'])){

                $pageNum=$_GET['page'];

        }

        $offset=($pageNum-1)*$rowsPerPage;

               

        $query="SELECT id FROM forum";

        $result=(mysql_query($query));

        $numrows=mysql_numrows($result);

       

        $maxPage = ceil($numrows/$rowsPerPage);



        $query="SELECT name, comment,email, date FROM forum ORDER BY id DESC LIMIT $offset, $rowsPerPage";

        $result=mysql_query($query) or die (mysql_error());

       

        if ($numrows < 1){

                $content.="<p>There are no comments.</p>\n";

        } else {

                while($row=mysql_fetch_assoc($result)){

                        $date = date('F d, Y', strtotime($row['date']));

                       

                        $content.='<div class="comment">'."\n";

                        $content.='<strong><a href="mailto:'.$row['email'].'">'.$row['name']."</a></strong><br />\n";

                        $content.=$row['comment']."<br />\n";

                        $content.='<span class="date">'.$date."</span>\n";

                        $content.="</div>\n";

                }

        }



        if ($pageNum > 1){

                $page=$pageNum - 1;

                $prev='<a href="forum.php?page='.$page.'">Previous</a>';

        } else {

                $prev="Previous";

        }



        if ($pageNum < $maxPage){

                $page=$pageNum + 1;

                $next='<a href="forum.php?page='.$page.'">Next</a>';

        } else { $next="Next";}

       

        $content.="<p>$prev | $next<br />\n";

        $content.="Page $pageNum of $maxPage.</p>\n";

       

        require 'library/template.php';

?>


Top
 Profile  
 
 Post subject:
PostPosted: Tue Aug 21, 2007 1:17 pm 
Offline
Neighborhood Spidermoddy
User avatar

Joined: Mon Mar 29, 2004 4:24 pm
Posts: 31559
Location: Bothell, Washington, USA
Here's the quick list of things I see:
  • Use of eregi() over, preg_match(). Speed and future compatibility issue.
  • Assumption that $_POST['submit'] will always be present.. it doesn't have to be. $_SERVER['REQUEST_METHOD'].
  • Comments could contain HTML thereby allowing anyone to inject page code. Naughty!
  • header() based redirection without full URL, http:// and all. Standards compliance issue.
  • Appears to produce invalid, non-compliant HTML.
  • Selection for post count could be modified to use COUNT() reducing the database needs.
  • Page could be used as a spam harvester. Email addresses being shown.
  • The date could be shaped during selection instead of going through strtotime().
  • Potential reserved word collision in the use of "date" and "comment" for column names. It may not apply to you right now, but it probably will when you move this to another server.


Top
 Profile  
 
 Post subject:
PostPosted: Tue Aug 21, 2007 2:31 pm 
Offline
Forum Contributor
User avatar

Joined: Thu Nov 10, 2005 1:31 pm
Posts: 355
Location: Broolyn, NY
Thanks for taking a look. I have a few questions based on your comments:

1) I don't follow:
Quote:
Assumption that $_POST['submit'] will always be present.. it doesn't have to be. $_SERVER['REQUEST_METHOD']

2) In terms of using count(), are you referring to this piece of code?
Syntax: [ Download ] [ Hide ]
$query="SELECT id FROM forum";
$result=(mysql_query($query));
$numrows=mysql_numrows($result);

Are you saying to do this? Instead of using the PHP function? Let MySQL do the work?
Syntax: [ Download ] [ Hide ]
$query="COUNT(id) as numrows FROM forum";

3) This sounds great, could you give an example?
Quote:
The date could be shaped during selection instead of going through strtotime().


Top
 Profile  
 
 Post subject:
PostPosted: Tue Aug 21, 2007 2:40 pm 
Offline
Neighborhood Spidermoddy
User avatar

Joined: Mon Mar 29, 2004 4:24 pm
Posts: 31559
Location: Bothell, Washington, USA
  1. Many browsers do not require interacting with the submit button to submit a form. Some don't send the submit button when this happens. Your code requires that someone interacts with your submit button. It's not a very good user experience.
  2. Ignoring the SQL syntax error, yes.
  3. Look up DATE_FORMAT() in the MySQL manual.


Top
 Profile  
 
 Post subject:
PostPosted: Tue Aug 21, 2007 3:00 pm 
Offline
Forum Contributor
User avatar

Joined: Thu Nov 10, 2005 1:31 pm
Posts: 355
Location: Broolyn, NY
Thanks, I understand it all but #1. How else would someone interact with the form? Return Key? Isn't that what the submit button is for?


Top
 Profile  
 
 Post subject:
PostPosted: Tue Aug 21, 2007 3:08 pm 
Offline
Neighborhood Spidermoddy
User avatar

Joined: Mon Mar 29, 2004 4:24 pm
Posts: 31559
Location: Bothell, Washington, USA
Submit the form using IE through one of the text input elements. You'll get your errors.


Top
 Profile  
 
 Post subject:
PostPosted: Tue Aug 21, 2007 3:22 pm 
Offline
Forum Contributor
User avatar

Joined: Thu Nov 10, 2005 1:31 pm
Posts: 355
Location: Broolyn, NY
Great. Thanks for all your help!


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 7 posts ] 

All times are UTC - 5 hours


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Jump to:  
Powered by phpBB® Forum Software © phpBB Group