Insecure PHP

[Ptsface12]

Hello,

          I'm trying to create a Content Management System (CMS) however, I always feel that my code is insecure and unprofessional. For example, here is some context from my core file:

       

       

 public function getSiteData() {
  $q = "SELECT * FROM siteSettings ORDER BY id DESC LIMIT 1";
  $r = mysql_query($q);
  
  while ( $a = mysql_fetch_assoc($r) ) {
  global $siteName, $siteMotto, $siteIco;
  $siteName = $a['site_name'];
  $siteMotto = $a['site_motto'];
  $siteIco = $a['site_ico'];
        }
  }

public function adminGet() {
    $q = "SELECT * FROM testDB ORDER BY created DESC LIMIT 4";
    $r = mysql_query($q);

    if ( $r !== false && mysql_num_rows($r) > 0 ) {
      while ( $a = mysql_fetch_assoc($r) ) {
        $title = stripslashes($a['title']);
        $bodytext = stripslashes(substr($a['bodytext'],0,100));

 

I seem to feel that the solutions that I approach towards my content management system are insecure, and could lead to easy hacks and injections. Now, when I compare my code to other content management systems, I seem to  see a huge difference in there professional layout, and the way they go about their coding.  I'm only 14, and have been studying PHP for around a year and a half now, I'm wanting to become a little more advanced in PHP, but I can't feel like I can due to my insecure coding. Would anybody be able to point me in the right direction, or tell me where I'm going wrong please?

 

I have MSN and Yahoo, if you'd prefer to contact me that way. MSN: [email protected] : Yahoo: Ptsface12.

 

Many thanks,

Jarrod

[ignace]

1) The while is unnecessary.

 

while ( $a = mysql_fetch_assoc($r) ) {
  global $siteName, $siteMotto, $siteIco;
  $siteName = $a['site_name'];
  $siteMotto = $a['site_motto'];
  $siteIco = $a['site_ico'];
        }

 

Instead just write:

 

$a = mysql_fetch_assoc($r);
global $siteName, $siteMotto, $siteIco;
$siteName = $a['site_name'];
$siteMotto = $a['site_motto'];
$siteIco = $a['site_ico'];

 

2) PHP has a built-in data-access layer called PHP Data Objects (PDO), use it! There are plans to ditch the mysql extension in favor of mysqli, so avoid it, if possible or use PDO. That said use PDO using the mysqli driver.

 

3) Create classes to group related functionality.

 

abstract class DataStoreService {
    private $pdo;
    public function __construct(PDO $pdo) {
        $this->pdo = $pdo;
    }
    protected function query($sql) {
        $stmt = $this->pdo->query($sql);
        if (false === $stmt) {
            throw new ErrorException($this->pdo->errorInfo(), $this->pdo->errorCode());
        }
        return $stmt;
    }
}
class Site {/*represents the site*/}
class SiteSettings extends DataStoreService {
    public function getSiteData() {
        $stmt = $this->query('SELECT * FROM siteSettings ORDER BY id DESC LIMIT 1');
        if ($stmt->rowCount()) {
            return $stmt->fetchObject( 'Site' );
        }
        return null; // no site data available! bad? throw an exception if it is.
    }
}
class Article {/*represents an article*/}
class Articles extends DataStoreService {
    public function getLatest() {
        $stmt = $this->query('SELECT * FROM testDB ORDER BY created DESC LIMIT 4');
        if ($stmt->rowCount()) {
            return $stmt->fetchAll( PDO::FETCH_CLASS, 'Article' );
        }
        return array(); // nothing here!
}

 

4) Avoid the use of the global keyword. You don't need it! Read up on Depedency Injection and Service Containers

[Ptsface12]

Hello!

        Woah thanks, but I don't get ANY of that. So, I'm guessing, that I'd then call the function by $stmt something like this?

 

new SiteSettings;
$stmt;

 

I'm guessing that is TERRIBLY wrong, I'll try and do some research on it, but from what I can gather, I can't understand hardly anything. I'm really only used to basic PHP