diff options
-rwxr-xr-x | admin/administration.php | 2 | ||||
-rw-r--r-- | admin/changepassword.php | 4 | ||||
-rwxr-xr-x | admin/index.php | 2 | ||||
-rwxr-xr-x | admin/login.php | 5 | ||||
-rw-r--r-- | admin/logout.php | 9 | ||||
-rwxr-xr-x | admin/subscriptions.php | 4 | ||||
-rwxr-xr-x | app/app.php | 4 | ||||
-rw-r--r-- | app/classes/CSRF.php | 55 | ||||
-rw-r--r-- | app/helpers.php | 57 | ||||
-rw-r--r-- | composer.json | 3 | ||||
-rw-r--r-- | composer.lock | 52 | ||||
-rw-r--r-- | tests/HelpersTest.php | 15 |
12 files changed, 203 insertions, 9 deletions
diff --git a/admin/administration.php b/admin/administration.php index 34afe73..26f6710 100755 --- a/admin/administration.php +++ b/admin/administration.php @@ -24,6 +24,7 @@ $page_content = <<<"FRAGMENT" <div class="widget"> <h3>{$l10n->getString('Clear cache')}</h3> <form action="purgecache.php" method="post" id="frmPurge"> + <input type="hidden" value="{$csrf->generate('frmPurge')}" name="_csrf"> <p><label>{$l10n->getString('Clear cache:')}</label><input type="submit" class="submit delete" name="purge" id="purge" value="{$l10n->getString('Clear')}" /></p> <p class="help">{$l10n->getString('Clearing the cache will make moonmoon reload all feeds.')}</p> </form> @@ -32,6 +33,7 @@ $page_content = <<<"FRAGMENT" <div class="widget"> <h3>{$l10n->getString('Change administrator password')}</h3> <form action="changepassword.php" method="post" id="frmPassword"> + <input type="hidden" value="{$csrf->generate('frmPassword')}" name="_csrf"> <p><label for="password">{$l10n->getString('New password:')}</label> <input type="password" class="text" value="" name="password" id="password" size="20" /> <input type="submit" class="submit delete" name="changepwd" id="changepwd" value="{$l10n->getString('Change password')}" /></p> </form> </div> diff --git a/admin/changepassword.php b/admin/changepassword.php index 8c38769..3b4500e 100644 --- a/admin/changepassword.php +++ b/admin/changepassword.php @@ -1,7 +1,9 @@ <?php + +require_once __DIR__.'/../app/app.php'; require_once __DIR__.'/inc/auth.inc.php'; -if (isset($_POST['password']) && ('' != $_POST['password'])){ +if ($csrf->verify($_POST['_csrf'], 'frmPassword') && isset($_POST['password']) && ('' != $_POST['password'])) { $out = '<?php $login="admin"; $password="'.md5($_POST['password']).'"; ?>'; file_put_contents(__DIR__.'/inc/pwd.inc.php', $out); die("Password changed. <a href='administration.php'>Login</a>"); diff --git a/admin/index.php b/admin/index.php index a01b77b..0118923 100755 --- a/admin/index.php +++ b/admin/index.php @@ -79,6 +79,7 @@ ob_start(); <input type="submit" class="submit add" name="add" value="<?=_g('Add Feed')?>" /> </fieldset> <p class="help"><?=_g('Accepted formats are RSS and ATOM. If the link is not a feed, moonmoon will try to autodiscover the feed.')?></p> + <input type="hidden" value="<?php echo $csrf->generate('feedmanage'); ?>" name="_csrf"> </form> </div> @@ -87,6 +88,7 @@ ob_start(); <form action="subscriptions.php" method="post" id="feedmanage"> <p class="action"> <span class="count"><?php echo sprintf(_g('Number of feeds: %s'), $count_feeds)?></span> + <input type="hidden" value="<?php echo $csrf->generate('feedmanage'); ?>" name="_csrf"> <input type="submit" class="submit save" name="save" id="save" value="<?=_g('Save changes')?>" /> <input type="submit" class="submit delete" name="delete" id="delete" value="<?=_g('Delete selected Feeds')?>" /> </p> diff --git a/admin/login.php b/admin/login.php index 3ba4d2b..a95e59f 100755 --- a/admin/login.php +++ b/admin/login.php @@ -1,10 +1,13 @@ <?php + +require_once __DIR__ . '/../app/app.php'; + if (isset($_POST['password'])) { + session_regenerate_id(); setcookie('auth',md5($_POST['password'])); header('Location: index.php'); } -require_once __DIR__ . '/../app/app.php'; $page_content = <<<FRAGMENT <form action="" method="post" class="login"> <fieldset> diff --git a/admin/logout.php b/admin/logout.php index 6dd32aa..adb843f 100644 --- a/admin/logout.php +++ b/admin/logout.php @@ -1,5 +1,10 @@ <?php + +require_once __DIR__ . '/../app/app.php'; + setcookie('auth','', time()-3600); +session_destroy(); +session_regenerate_id(); + header('Location: login.php'); -die; -?>
\ No newline at end of file +die(); diff --git a/admin/subscriptions.php b/admin/subscriptions.php index f63af8f..7b2fb6f 100755 --- a/admin/subscriptions.php +++ b/admin/subscriptions.php @@ -7,6 +7,10 @@ function removeSlashes(&$item, $key){ $item = stripslashes($item); } +if (!$csrf->verify($_POST['_csrf'], 'feedmanage')) { + die('Invalid CSRF token!'); +} + if (isset($_POST['opml']) || isset($_POST['add'])) { // Load old OPML diff --git a/app/app.php b/app/app.php index a6232cf..0797cc7 100755 --- a/app/app.php +++ b/app/app.php @@ -7,6 +7,8 @@ require_once __DIR__.'/../vendor/autoload.php'; $savedConfig = __DIR__.'/../custom/config.yml'; $moon_version = file_get_contents(__DIR__.'/../VERSION'); +session_start(); + if (is_installed()) { $conf = Spyc::YAMLLoad($savedConfig); @@ -27,4 +29,4 @@ if (is_installed()) { } $l10n = new Simplel10n($conf['locale']); - +$csrf = new CSRF(); diff --git a/app/classes/CSRF.php b/app/classes/CSRF.php new file mode 100644 index 0000000..639f573 --- /dev/null +++ b/app/classes/CSRF.php @@ -0,0 +1,55 @@ +<?php + +class CSRF +{ + /** @var string */ + const HMAC_ALGORITHM = 'sha1'; + + /** @var string */ + const SESSION_KEY_NAME = '_csrf_key'; + + /** + * Ensure that a CSRF token is valid for a given action. + * + * @param string $token + * @param string $action + * @return bool + */ + public static function verify($token = '', $action = null) + { + if (!is_string($token) || !is_string($action)) { + return false; + } + + $known = self::generate($action); + return hash_equals($known, $token); + } + + /** + * Generate a CSRF token for a given action. + * + * @param string $action + * @throws InvalidArgumentException + * @return string + */ + public static function generate($action = null) + { + if (!is_string($action)) { + throw InvalidArgumentException('A valid action must be defined.'); + } + return hash_hmac(self::HMAC_ALGORITHM, $action, self::getKey()); + } + + /** + * Get HMAC key. + * + * @return string + */ + public static function getKey() + { + if (empty($_SESSION[self::SESSION_KEY_NAME])) { + $_SESSION[self::SESSION_KEY_NAME] = random_bytes(16); + } + return $_SESSION[self::SESSION_KEY_NAME]; + } +} diff --git a/app/helpers.php b/app/helpers.php index 3bce65f..01d0086 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -1,6 +1,29 @@ <?php /** + * Register polyfills for old PHP versions. + * + * This way, the real function will only be called if it + * is available, and we won't force the use of our own + * implementation. + */ +function register_polyfills() +{ + if (!function_exists('hash_equals')) { + function hash_equals($known_string, $user_string) { + call_user_func_array('_hash_equals', func_get_args()); + } + } + + if (!function_exists('random_bytes')) { + // If this function does not exist, it will be exposed + // automatically by paragonie/random_compat. + } +} + +register_polyfills(); + +/** * Path to the _custom_ directory. * * @param string $file Append this filename to the returned path. @@ -72,4 +95,36 @@ function removeCustomFiles() unlink($path); } } -}
\ No newline at end of file +} + +/** + * Compare two strings in a constant-time manner. + * + * It returns `true` if both strings are exactly the same + * (same size and same value). + * + * @param string $known_string + * @param string $user_string + * @return bool + */ +function _hash_equals($known_string = '', $user_string = '') +{ + // In our case, it's not problematic if `$known_string`'s + // size leaks, we will only compare password hashes and + // CSRF tokens—their size is already somehow public. + if (!is_string($known_string) || !is_string($user_string) + || strlen($known_string) !== strlen($user_string)) { + return false; + } + + $ret = 0; + + // Do not stop the comparison when a difference is found, + // always completely compare them. + for ($i = 0; $i < strlen($known_string); $i++) { + $ret |= (ord($known_string[$i]) ^ ord($user_string[$i])); + } + + return !$ret; +} + diff --git a/composer.json b/composer.json index c43b48e..ea5032d 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,8 @@ "require": { "php": "^5.6 || ^7.0", "mustangostang/spyc": "0.5.1", - "simplepie/simplepie": "^1.5" + "simplepie/simplepie": "^1.5", + "paragonie/random_compat": "^2.0" }, "require-dev": { "guzzlehttp/guzzle": "^6.3", diff --git a/composer.lock b/composer.lock index 1d3cf2e..7959c9c 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "content-hash": "381ab1da48dd363669e218a50e19f0b7", + "content-hash": "a4fc919a4e8ef2463ff4a336940bc993", "packages": [ { "name": "mustangostang/spyc", @@ -54,6 +54,54 @@ "time": "2013-02-21T10:52:01+00:00" }, { + "name": "paragonie/random_compat", + "version": "v2.0.11", + "source": { + "type": "git", + "url": "https://github.com/paragonie/random_compat.git", + "reference": "5da4d3c796c275c55f057af5a643ae297d96b4d8" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/paragonie/random_compat/zipball/5da4d3c796c275c55f057af5a643ae297d96b4d8", + "reference": "5da4d3c796c275c55f057af5a643ae297d96b4d8", + "shasum": "" + }, + "require": { + "php": ">=5.2.0" + }, + "require-dev": { + "phpunit/phpunit": "4.*|5.*" + }, + "suggest": { + "ext-libsodium": "Provides a modern crypto API that can be used to generate random bytes." + }, + "type": "library", + "autoload": { + "files": [ + "lib/random.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Paragon Initiative Enterprises", + "email": "security@paragonie.com", + "homepage": "https://paragonie.com" + } + ], + "description": "PHP 5.x polyfill for random_bytes() and random_int() from PHP 7", + "keywords": [ + "csprng", + "pseudorandom", + "random" + ], + "time": "2017-09-27T21:40:39+00:00" + }, + { "name": "simplepie/simplepie", "version": "1.5", "source": { @@ -1543,7 +1591,7 @@ "minimum-stability": "stable", "stability-flags": [], "prefer-stable": false, - "prefer-lowest": true, + "prefer-lowest": false, "platform": { "php": "^5.6 || ^7.0" }, diff --git a/tests/HelpersTest.php b/tests/HelpersTest.php new file mode 100644 index 0000000..141e604 --- /dev/null +++ b/tests/HelpersTest.php @@ -0,0 +1,15 @@ +<?php + +use PHPUnit\Framework\TestCase; + +class HelpersTest extends TestCase +{ + function test_constant_time_compare() + { + $this->assertTrue(_hash_equals('abc', 'abc')); + $this->assertFalse(_hash_equals('abc', 'ab')); + $this->assertFalse(_hash_equals('ab', 'abc')); + $this->assertFalse(_hash_equals('abcd', 'adbc')); + $this->assertFalse(_hash_equals(0, 0)); + } +} |