From a362ac12b119b49519f5af51b92ebb7d6e127b87 Mon Sep 17 00:00:00 2001
From: Simon Rettberg
Date: Thu, 31 Oct 2013 12:38:25 +0100
Subject: Comments, minor refactoring, possiblity to validate configuration
 parameters

---
 inc/crypto.inc.php    | 29 ++++++++++++++++++
 inc/db.inc.php        | 27 +++++++++++++++-
 inc/message.inc.php   | 85 +++++++++++++++++++++++++++++++++++++++------------
 inc/user.inc.php      |  2 +-
 inc/util.inc.php      | 44 +++++++++++++++++++++-----
 inc/validator.inc.php | 43 ++++++++++++++++++++++++++
 6 files changed, 202 insertions(+), 28 deletions(-)
 create mode 100644 inc/crypto.inc.php
 create mode 100644 inc/validator.inc.php

(limited to 'inc')

diff --git a/inc/crypto.inc.php b/inc/crypto.inc.php
new file mode 100644
index 00000000..54cdef8a
--- /dev/null
+++ b/inc/crypto.inc.php
@@ -0,0 +1,29 @@
+<?php
+
+class Crypto
+{
+
+	/**
+	 * Hash given string using crypt's $6$,
+	 * which translates to ~130 bit salt
+	 * and 5000 rounds of hashing with SHA-512.
+	 */
+	public static function hash6($password)
+	{
+		$salt = substr(str_replace('+', '.', base64_encode(pack('N4', mt_rand(), mt_rand(), mt_rand(), mt_rand()))), 0, 22);
+		$hash = crypt($password, '$6$' . $salt);
+		if (strlen($hash) < 60) Util::traceError('Error hashing password using SHA-512');
+		return $hash;
+	}
+
+	/**
+	 * Check if the given password matches the given cryp hash.
+	 * Useful for checking a hashed password.
+	 */
+	public static function verify($password, $hash)
+	{
+		return crypt($password, $hash) === $hash;
+	}
+
+}
+
diff --git a/inc/db.inc.php b/inc/db.inc.php
index 09341a07..a797ae93 100644
--- a/inc/db.inc.php
+++ b/inc/db.inc.php
@@ -1,11 +1,18 @@
 <?php
 
+/**
+ * Handle communication with the database
+ * This is a very thin layer between you and PDO.
+ */
 class Database
 {
 	private static $dbh = false;
 	private static $statements = array();
 
-	public static function init()
+	/**
+	 * Connect to the DB if not already connected.
+	 */
+	private static function init()
 	{
 		if (self::$dbh !== false) return;
 		try {
@@ -15,6 +22,10 @@ class Database
 		}
 	}
 
+	/**
+	 * If you just need the first row of a query you can use this.
+	 * Will return an associative array, or false if no row matches the query
+	 */
 	public static function queryFirst($query, $args = array())
 	{
 		$res = self::simpleQuery($query, $args);
@@ -22,6 +33,10 @@ class Database
 		return $res->fetch(PDO::FETCH_ASSOC);
 	}
 
+	/**
+	 * Execute the given query and return the number of rows affected.
+	 * Mostly useful for UPDATEs or INSERTs
+	 */
 	public static function exec($query, $args = array())
 	{
 		$res = self::simpleQuery($query, $args);
@@ -29,6 +44,12 @@ class Database
 		return $res->rowCount();
 	}
 
+	/**
+	 * Execute the given query and return the corresponding PDOStatement object
+	 * Note that this will re-use PDOStatements, so if you run the same
+	 * query again with different params, do not rely on the first PDOStatement
+	 * still being valid. If you need to do something fancy, use Database::prepare
+	 */
 	public static function simpleQuery($query, $args = array())
 	{
 		self::init();
@@ -44,6 +65,10 @@ class Database
 		return self::$statements[$query];
 	}
 
+	/**
+	 * Simply calls PDO::prepare and returns the PDOStatement.
+	 * You must call PDOStatement::execute manually on it.
+	 */
 	public static function prepare($query)
 	{
 		self:init();
diff --git a/inc/message.inc.php b/inc/message.inc.php
index b90ed630..4da277e7 100644
--- a/inc/message.inc.php
+++ b/inc/message.inc.php
@@ -6,10 +6,12 @@ $error_text = array(
 	'token'             => 'Ungültiges Token. CSRF Angriff?',
 	'adduser-disabled'  => 'Keine ausreichenden Rechte, um weitere Benutzer hinzuzufügen',
 	'password-mismatch' => 'Passwort und Passwortbestätigung stimmen nicht überein',
-	'empty-field'       => 'Ein benötigtes Feld wurde nicht ausgefüllt',
+	'empty-field'       => 'Ein Feld wurde nicht ausgefüllt',
 	'adduser-success'   => 'Benutzer erfolgreich hinzugefügt',
 	'no-permission'     => 'Keine ausreichenden Rechte, um auf diese Seite zuzugreifen',
 	'settings-updated'  => 'Einstellungen wurden aktualisiert',
+	'debug-mode'        => 'Der Debug-Modus ist aktiv!',
+	'value-invalid'     => 'Der Wert {{1}} ist ungültig für die Option {{0}} und wurde ignoriert',
 );
 
 class Message
@@ -17,51 +19,96 @@ class Message
 	private static $list = array();
 	private static $flushed = false;
 
+	/**
+	 * Add error message to page. If messages have not been flushed
+	 * yet, it will be added to the queue, otherwise it will be added
+	 * in place during rendering.
+	 */
 	public static function addError($id)
 	{
-		self::$list[] = array(
-			'type' => 'error',
-			'id'    => $id
-		);
-		if (self::$flushed) self::renderList();
+		self::add('error', $id, func_get_args());
 	}
 
 	public static function addWarning($id)
 	{
-		self::$list[] = array(
-			'type' => 'warning',
-			'id'    => $id
-		);
-		if (self::$flushed) self::renderList();
+		self::add('warning', $id, func_get_args());
 	}
 
 	public static function addInfo($id)
 	{
-		self::$list[] = array(
-			'type' => 'info',
-			'id'    => $id
-		);
-		if (self::$flushed) self::renderList();
+		self::add('info', $id, func_get_args());
 	}
 
 	public static function addSuccess($id)
 	{
+		self::add('success', $id, func_get_args());
+	}
+
+	/**
+	 * Internal function that adds a message. Used by
+	 * addError/Success/Info/... above.
+	 */
+	private static function add($type, $id, $params)
+	{
+		global $error_text;
+		if (!isset($error_text[$id])) Util::traceError('Invalid message id: ' . $id);
 		self::$list[] = array(
-			'type' => 'success',
-			'id'    => $id
+			'type'   => $type,
+			'id'     => $id,
+			'params' => array_slice($params, 1)
 		);
 		if (self::$flushed) self::renderList();
 	}
 
+	/**
+	 * Render all currently queued messages, flushing the queue.
+	 * After calling this, any further calls to add* will be rendered in
+	 * place in the current page output.
+	 */
 	public static function renderList()
 	{
 		global $error_text;
 		foreach (self::$list as $item) {
-			Render::addTemplate('messagebox-' . $item['type'], array('message' => $error_text[$item['id']]));
+			$message = $error_text[$item['id']];
+			foreach ($item['params'] as $index => $text) {
+				$message = str_replace('{{' . $index . '}}', $text, $message);
+			}
+			Render::addTemplate('messagebox-' . $item['type'], array('message' => $message));
 		}
 		self::$list = array();
 		self::$flushed = true;
 	}
 
+	/**
+	 * Deserialize any messages from the current HTTP request and
+	 * place them in the message queue.
+	 */
+	public static function fromRequest()
+	{
+		$messages = is_array($_REQUEST['message']) ? $_REQUEST['message'] : array($_REQUEST['message']);
+		foreach ($messages as $message) {
+			$data = explode('|', $message);
+			if (count($data) < 2 || !preg_match('/^(error|warning|info|success)$/', $data[0])) continue;
+			self::add($data[0], $data[1], array_slice($data, 1));
+		}
+	}
+
+	/**
+	 * Turn the current message queue into a serialized version,
+	 * suitable for appending to a GET or POST request
+	 */
+	public static function toRequest()
+	{
+		$parts = array();
+		foreach (self::$list as $item) {
+			$str = 'message[]=' . urlencode($item['type'] . '|' .$item['id']);
+			if (!empty($item['params'])) {
+				$str .= '|' . implode('|', $item['params']);
+			}
+			$parts[] = $str;
+		}
+		return implode('&', $parts);
+	}
+
 }
 
diff --git a/inc/user.inc.php b/inc/user.inc.php
index 38e57e33..b333c7e4 100644
--- a/inc/user.inc.php
+++ b/inc/user.inc.php
@@ -39,7 +39,7 @@ class User
 	{
 		$ret = Database::queryFirst('SELECT userid, passwd FROM user WHERE login = :user LIMIT 1', array(':user' => $user));
 		if ($ret === false) return false;
-		if (crypt($pass, $ret['passwd']) !== $ret['passwd']) return false;
+		if (!Crypto::verify($pass, $ret['passwd'])) return false;
 		Session::create();
 		Session::set('uid', $ret['userid']);
 		Session::set('token', md5(rand() . time() . rand() . $_SERVER['REMOTE_ADDR'] . rand() . $_SERVER['REMOTE_PORT'] . rand() . $_SERVER['HTTP_USER_AGENT']));
diff --git a/inc/util.inc.php b/inc/util.inc.php
index 0d85b989..f456d164 100644
--- a/inc/util.inc.php
+++ b/inc/util.inc.php
@@ -1,30 +1,53 @@
 <?php
 
-$verboseDebug = true;
-
 class Util
 {
+
+	/**
+	 * Displays an error message and stops script execution.
+	 * If CONFIG_DEBUG is true, it will also dump a stack trace
+	 * and all globally defined variables.
+	 * (As this might reveal sensistive data you should never enable it in production)
+	 */
 	public static function traceError($message)
 	{
-		global $verboseDebug;
 		Header('Content-Type: text/plain; charset=utf-8');
 		echo "--------------------\nFlagrant system error:\n$message\n--------------------\n\n";
-		if (isset($verboseDebug) && $verboseDebug) {
+		if (defined('CONFIG_DEBUG') && CONFIG_DEBUG) {
 			debug_print_backtrace();
 			echo "\n\n";
-			$vars = get_defined_vars();
-			print_r($vars);
+			print_r($GLOBALS);
 		}
 		exit(0);
 	}
 
+	/**
+	 * Redirects the user via a '302 Moved' header.
+	 * An active session will be saved, any messages that haven't
+	 * been displayed yet will be appended to the redirect.
+	 */
 	public static function redirect($location)
 	{
 		Session::save();
+		$messages = Message::toRequest();
+		if (!empty($messages)) {
+			if (strpos($location, '?') === false) {
+				$location .= '?' . $messages;
+			} else {
+				$location .= '&' . $messages;
+			}
+		}
 		Header('Location: ' . $location);
 		exit(0);
 	}
 
+	/**
+	 * Verify the user's token that protects agains CSRF.
+	 * If the user is logged in and there is no token variable set in
+	 * the request, or the submitted token does not match the user's
+	 * token, this function will return false and display an error.
+	 * If the token matches, or the user is not logged in, it will return true.
+	 */
 	public static function verifyToken()
 	{
 		if (Session::get('token') === false) return true;
@@ -33,7 +56,14 @@ class Util
 		return false;
 	}
 
-	function markup($string)
+	/**
+	 * Simple markup "rendering":
+	 * *word* is bold
+	 * /word/ is italics
+	 * _word_ is underlined
+	 * \n is line break
+	 */
+	public static function markup($string)
 	{
 		$string = htmlspecialchars($string);
 		$string = preg_replace('#(^|[\n \-_/\.])\*(.+?)\*($|[ \-_/\.\!\?,])#is', '$1<b>$2</b>$3', $string);
diff --git a/inc/validator.inc.php b/inc/validator.inc.php
new file mode 100644
index 00000000..72b7fa0b
--- /dev/null
+++ b/inc/validator.inc.php
@@ -0,0 +1,43 @@
+<?php
+
+/**
+ * This class contains all the helper functions that
+ * can be referenced by a config setting. Every function
+ * here is supposed to validate the given config value
+ * and wither return the validated and possibly sanitized
+ * value, or false to indicate that the given value is invalid.
+ */
+class Validator
+{
+
+	public static function validate($condition, $value)
+	{
+		if (empty($condition)) return $value;
+		$data = explode(':', $condition, 2);
+		switch ($data[0]) {
+		case 'regex':
+			if (preg_match($data[1], $value)) return $value;
+			return false;
+		case 'function':
+			return self::$data[1]($value);
+		default:
+			Util::traceError('Unknown validation method: ' . $data[0]);
+		}
+	}
+
+	/**
+	 * Validate linux password. If already in $6$ hash form,
+	 * the unchanged value will be returned.
+	 * if empty, an empty string will also be returned.
+	 * Otherwise it it assumed that the value is a plain text
+	 * password that is supposed to be hashed.
+	 */
+	private static function linuxPassword($value)
+	{
+		if (empty($value)) return '';
+		if (preg_match('/^\$6\$.+\$./', $value)) return $value;
+		return Crypto::hash6($value);
+	}
+
+}
+
-- 
cgit v1.2.3-55-g7522