Forum project using OOP PHP

Question

I've been developing a forum as my first OOP project, I've successfully built a login/registration and other functionality typical in forums.

I'm looking for some guidance in terms of where to take the build and other common functions that will need integrating.

users.php

<?php
class USER
{
  private $db;

  function __construct($DB_con)
  {
    $this->db = $DB_con;
  }

  public function register($fname,$lname,$uname,$umail,$upass)
  {
   try
   {
     $new_password = password_hash($upass, PASSWORD_DEFAULT);

     $stmt = $this->db->prepare("INSERT INTO users(user_name,user_email,user_pass) 
       VALUES(:uname, :umail, :upass)");

     $stmt->bindparam(":uname", $uname);
     $stmt->bindparam(":umail", $umail);
     $stmt->bindparam(":upass", $new_password);            
     $stmt->execute(); 

     return $stmt; 
   }
   catch(PDOException $e)
   {
     echo $e->getMessage();
   }    
 }

 public function login($uname,$umail,$upass)
 {
   try
   {
    $stmt = $this->db->prepare("SELECT * FROM users WHERE user_name=:uname OR user_email=:umail LIMIT 1");
    $stmt->execute(array(':uname'=>$uname, ':umail'=>$umail));
    $userRow=$stmt->fetch(PDO::FETCH_ASSOC);
    if($stmt->rowCount() > 0)
    {
     if(password_verify($upass, $userRow['user_pass']))
     {
      $_SESSION['user_session'] = $userRow['user_id'];
      return true;
    }
    else
    {
      return false;
    }
  }
}
catch(PDOException $e)
{
 echo $e->getMessage();
}
}

public function is_loggedin()
{
  if(isset($_SESSION['user_session']))
  {
   return true;
 }
}

public function redirect($url)
{
 header("Location: $url");
}

public function logout()
{
  session_destroy();
  unset($_SESSION['user_session']);
  return true;
}

public function back() {
  $previous = $_SERVER['HTTP_REFERER']; 
  if (isset($previous)) {
    echo $previous;
  } else {
    return FALSE;
  }


}

}
?>

forum.php

<?php 

include_once 'database.php';

class FORUM {

    function __construct($DB_con)
    {
        $this->db = $DB_con;
    }

    public function getColumn() {
        try {
            $query = $this->db->prepare("DESCRIBE posts");
            $query->execute();
        //retrieve the columns inside the table posts
            $forumrows = $query->fetchAll(PDO::FETCH_COLUMN); 
        //Output each column  Value
            foreach($forumrows as $forumrow) {
                echo $forumrow . "</br>";
            }
        } catch(PDOException $e) {
            echo $e->getMessage();
        }
    }

public function getColumnUrl() {

}

} //End forum class

$forum = new FORUM($DB_con);
$forum->getColumn();

?>

database.php

<?php

session_start();

$DB_host = "localhost";
$DB_user = "root";
$DB_pass = "";
$DB_name = "dblogin";

try
{
    $DB_con = new PDO("mysql:host={$DB_host};dbname={$DB_name}",$DB_user,$DB_pass);
    $DB_con->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch(PDOException $e)
{
    echo $e->getMessage();
}


include_once 'users.php';
$user = new USER($DB_con);

?>

The rest of the pages just optimise the the functions and a general bootstrap layout for the posts and login pages.

Answer 1

• back: I'm not sure what this function is used for (and the name doesn't seem that great), but it is open to XSS. An attacker could create a page at http://evil.com/?<img src=onerror=alert(1)>, which would redirect the user to your back page, which would then echo the XSS payload.
• you should always die after a header redirect (clients do not have to follow it, so any code after the redirect may be executed, which may have security implications).
• your indentation is inconsistent, and 1 or 2 spaces aren't really enough. I would use 4 at a minimum to ensure readability.
• your spacing is also off, and your positioning of curly brackets isn't consistent. You can easily solve all these formatting issues with any IDE.
• try to always write out your variable names. Right now, fname may be clear to you (in context), but firstName is much easier to read.
• a nested if-else in an if is often not so nice to read (especially with your formatting). It's difficult to see what happens when rowCount is not greater than 0. You could just combine the two ifs.
• getColumn doesn't actually get anything, it echoes something. You could rename it to showColumn, but really, it should return an array of columns, which is then echoed in a view class.
• the same goes for all the exceptions. Just catching them and echoing the message isn't a great way to handle it. If you can't recover in the function, just throw them upwards, and let the calling code decide what to do.