From e5947db7f132cd689ab2574e81ede52b0d9ff57b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Guillot?= Date: Fri, 7 Nov 2014 21:44:20 -0500 Subject: [PATCH] Add CSRF protections --- assets/css/app.css | 2 +- controllers/config.php | 40 ++++++++++++++++++++++++++-------------- controllers/feed.php | 10 ++++++++-- controllers/user.php | 7 +++++-- models/config.php | 37 +++++++++++++++++++++++++++++++++++++ templates/about.php | 4 ++-- templates/add.php | 3 +++ templates/api.php | 2 +- templates/config.php | 2 ++ templates/login.php | 2 ++ templates/new_db.php | 6 ++++++ 11 files changed, 93 insertions(+), 22 deletions(-) diff --git a/assets/css/app.css b/assets/css/app.css index 7526a10..45ffd42 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -16,6 +16,7 @@ body { body { margin: 0 auto; + margin-bottom: 30px; max-width: 780px; color: #333; font-family: "Helvetica Neue", Helvetica, Arial, sans-serif; @@ -375,7 +376,6 @@ nav .active a { font-size: 90%; display: inline; padding-right: 5px; - border-right: 1px dotted #ccc; } .page-header li:last-child { diff --git a/controllers/config.php b/controllers/config.php index 0388323..e773b14 100644 --- a/controllers/config.php +++ b/controllers/config.php @@ -14,13 +14,15 @@ Router\get_action('new-db', function() { Response\html(Template\layout('new_db', array( 'errors' => array(), - 'values' => array(), + 'values' => array( + 'csrf' => Model\Config\generate_csrf(), + ), 'menu' => 'config', 'title' => t('New database') ))); } - Response\redirect('?action=config'); + Response\redirect('?action=about'); }); // Create a new database @@ -29,6 +31,7 @@ Router\post_action('new-db', function() { if (ENABLE_MULTIPLE_DB) { $values = Request\values(); + Model\Config\check_csrf_values($values); list($valid, $errors) = Model\Database\validate($values); if ($valid) { @@ -40,18 +43,18 @@ Router\post_action('new-db', function() { Session\flash_error(t('Unable to create the new database.')); } - Response\redirect('?action=config'); + Response\redirect('?action=about'); } Response\html(Template\layout('new_db', array( 'errors' => $errors, - 'values' => $values, + 'values' => $values + array('csrf' => Model\Config\generate_csrf()), 'menu' => 'config', 'title' => t('New database') ))); } - Response\redirect('?action=config'); + Response\redirect('?action=about'); }); // Auto-update @@ -73,22 +76,30 @@ Router\get_action('auto-update', function() { // Re-generate tokens Router\get_action('generate-tokens', function() { - Model\Config\new_tokens(); + if (Model\Config\check_csrf(Request\param('csrf'))) { + Model\Config\new_tokens(); + } + Response\redirect('?action=api'); }); // Optimize the database manually Router\get_action('optimize-db', function() { - Database::get('db')->getConnection()->exec('VACUUM'); - Response\redirect('?action=config'); + if (Model\Config\check_csrf(Request\param('csrf'))) { + Database::get('db')->getConnection()->exec('VACUUM'); + } + + Response\redirect('?action=about'); }); // Download the compressed database Router\get_action('download-db', function() { - Response\force_download('db.sqlite.gz'); - Response\binary(gzencode(file_get_contents(\Model\Database\get_path()))); + if (Model\Config\check_csrf(Request\param('csrf'))) { + Response\force_download('db.sqlite.gz'); + Response\binary(gzencode(file_get_contents(Model\Database\get_path()))); + } }); // Display preferences page @@ -96,8 +107,7 @@ Router\get_action('config', function() { Response\html(Template\layout('config', array( 'errors' => array(), - 'values' => Model\Config\get_all(), - 'db_size' => filesize(\Model\Database\get_path()), + 'values' => Model\Config\get_all() + array('csrf' => Model\Config\generate_csrf()), 'languages' => Model\Config\get_languages(), 'timezones' => Model\Config\get_timezones(), 'autoflush_options' => Model\Config\get_autoflush_options(), @@ -115,6 +125,7 @@ Router\get_action('config', function() { Router\post_action('config', function() { $values = Request\values() + array('nocontent' => 0); + Model\Config\check_csrf_values($values); list($valid, $errors) = Model\Config\validate_modification($values); if ($valid) { @@ -131,8 +142,7 @@ Router\post_action('config', function() { Response\html(Template\layout('config', array( 'errors' => $errors, - 'values' => Model\Config\get_all(), - 'db_size' => filesize(\Model\Database\get_path()), + 'values' => Model\Config\get_all() + array('csrf' => Model\Config\generate_csrf()), 'languages' => Model\Config\get_languages(), 'timezones' => Model\Config\get_timezones(), 'autoflush_options' => Model\Config\get_autoflush_options(), @@ -160,6 +170,7 @@ Router\get_action('help', function() { Router\get_action('about', function() { Response\html(Template\layout('about', array( + 'csrf' => Model\Config\generate_csrf(), 'config' => Model\Config\get_all(), 'db_size' => filesize(\Model\Database\get_path()), 'menu' => 'config', @@ -171,6 +182,7 @@ Router\get_action('about', function() { Router\get_action('api', function() { Response\html(Template\layout('api', array( + 'csrf' => Model\Config\generate_csrf(), 'config' => Model\Config\get_all(), 'menu' => 'config', 'title' => t('API') diff --git a/controllers/feed.php b/controllers/feed.php index d907f39..9a6230d 100644 --- a/controllers/feed.php +++ b/controllers/feed.php @@ -136,7 +136,9 @@ Router\get_action('feeds', function() { Router\get_action('add', function() { Response\html(Template\layout('add', array( - 'values' => array(), + 'values' => array( + 'csrf' => Model\Config\generate_csrf(), + ), 'errors' => array(), 'menu' => 'feeds', 'title' => t('New subscription') @@ -148,6 +150,7 @@ Router\action('subscribe', function() { if (Request\is_post()) { $values = Request\values(); + Model\Config\check_csrf_values($values); $url = isset($values['url']) ? $values['url'] : ''; } else { @@ -173,7 +176,10 @@ Router\action('subscribe', function() { } Response\html(Template\layout('add', array( - 'values' => array('url' => $url), + 'values' => array( + 'url' => $url, + 'csrf' => Model\Config\generate_csrf(), + ), 'menu' => 'feeds', 'title' => t('Subscriptions') ))); diff --git a/controllers/user.php b/controllers/user.php index ea9a686..afdcdca 100644 --- a/controllers/user.php +++ b/controllers/user.php @@ -23,7 +23,9 @@ Router\get_action('login', function() { Response\html(Template\load('login', array( 'errors' => array(), - 'values' => array(), + 'values' => array( + 'csrf' => Model\Config\generate_csrf(), + ), 'databases' => Model\Database\get_list(), 'current_database' => Model\Database\select() ))); @@ -33,6 +35,7 @@ Router\get_action('login', function() { Router\post_action('login', function() { $values = Request\values(); + Model\Config\check_csrf_values($values); list($valid, $errors) = Model\User\validate_login($values); if ($valid) { @@ -41,7 +44,7 @@ Router\post_action('login', function() { Response\html(Template\load('login', array( 'errors' => $errors, - 'values' => $values, + 'values' => $values + array('csrf' => Model\Config\generate_csrf()), 'databases' => Model\Database\get_list(), 'current_database' => Model\Database\select() ))); diff --git a/models/config.php b/models/config.php index 225271d..4a35600 100644 --- a/models/config.php +++ b/models/config.php @@ -156,6 +156,43 @@ function get_nothing_to_read_redirections() ); } +// Create a CSRF token +function generate_csrf() +{ + if (empty($_SESSION['csrf'])) { + $_SESSION['csrf'] = array(); + } + + $token = generate_token(); + $_SESSION['csrf'][$token] = true; + + return $token; +} + +// Check CSRF token (form values) +function check_csrf_values(array &$values) +{ + if (empty($values['csrf']) || ! isset($_SESSION['csrf'][$values['csrf']])) { + $values = array(); + } + else { + + unset($_SESSION['csrf'][$values['csrf']]); + unset($values['csrf']); + } +} + +// Check CSRF token +function check_csrf($token) +{ + if (isset($_SESSION['csrf'][$token])) { + unset($_SESSION['csrf'][$values['csrf']]); + return true; + } + + return false; +} + // Generate a token from /dev/urandom or with uniqid() if open_basedir is enabled function generate_token() { diff --git a/templates/about.php b/templates/about.php index f82b5a2..99bf70b 100644 --- a/templates/about.php +++ b/templates/about.php @@ -22,8 +22,8 @@

diff --git a/templates/config.php b/templates/config.php index f3dfdc2..09a8c28 100644 --- a/templates/config.php +++ b/templates/config.php @@ -9,6 +9,8 @@
+ +
diff --git a/templates/login.php b/templates/login.php index f1a548b..1bd9b98 100644 --- a/templates/login.php +++ b/templates/login.php @@ -26,6 +26,8 @@ + +
diff --git a/templates/new_db.php b/templates/new_db.php index 67102fa..3a80fcd 100644 --- a/templates/new_db.php +++ b/templates/new_db.php @@ -2,10 +2,16 @@

+ + +