Internal Server Error with stack trace when trying to change password twice from forgot-password email

[mihailstoynov]

When trying to change password twice from forgot-password email you get an Internal Server Error with stack trace that contains scary info

How to reproduce

• Click on forgot password, request email
• Change password
• Go back to email and click on it again.
• You get a HTTP 500

Result

Internal Server Error
The server was unable to complete your request.

If this happens again, please send the technical details below to the server administrator.

More details can be found in the server log.

Technical details
Remote Address: INTERNAL IP
Request ID: NMb5lwhzn5tPD1XYP81Y
Type: TypeError
Code: 0
Message: Argument 1 passed to OC\Security\Crypto::decrypt() must be of the type string, null given, called in /nextcloud_install_dir//core/Controller/LostController.php on line 189
File: /nextcloud_install_dir//lib/private/Security/Crypto.php
Line: 113

Trace
#0 /nextcloud_install_dir//core/Controller/LostController.php(189): OC\Security\Crypto->decrypt(NULL, 'user@gmail.comrf...')
#1 /nextcloud_install_dir//core/Controller/LostController.php(155): OC\Core\Controller\LostController->checkPasswordResetToken('kjo34HAK4...', 'USERNAME')
#2 /nextcloud_install_dir//lib/private/AppFramework/Http/Dispatcher.php(166): OC\Core\Controller\LostController->resetform('ji9397398...', 'username')
#3 /nextcloud_install_dir//lib/private/AppFramework/Http/Dispatcher.php(99): OC\AppFramework\Http\Dispatcher->executeController(Object(OC\Core\Controller\LostController), 'resetform')
#4 /nextcloud_install_dir//lib/private/AppFramework/App.php(126): OC\AppFramework\Http\Dispatcher->dispatch(Object(OC\Core\Controller\LostController), 'resetform')
#5 /nextcloud_install_dir//lib/private/AppFramework/Routing/RouteActionHandler.php(47): OC\AppFramework\App::main('OC\\Core\\Control...', 'resetform', Object(OC\AppFramework\DependencyInjection\DIContainer), Array)
#6 [internal function]: OC\AppFramework\Routing\RouteActionHandler->__invoke(Array)
#7 /nextcloud_install_dir//lib/private/Route/Router.php(297): call_user_func(Object(OC\AppFramework\Routing\RouteActionHandler), Array)
#8 /nextcloud_install_dir//lib/base.php(975): OC\Route\Router->match('/lostpassword/r...')
#9 /nextcloud_install_dir//index.php(42): OC::handleRequest()
#10 {main}

Information disclosed

• username
• internal server ip ( that scared me a little)
• paths (/nextcloud_install_dir/)
• email
• part of the token

[kesselb]

If the token is invalid or expired there should be a warning like above but its broken if there is no encrypted token. null is not a valid argument for the decrypt method. We need to check $encryptedToken for null before we pass it to decrypt.

Index: core/Controller/LostController.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- core/Controller/LostController.php	(revision 3a99ef30dfa6a49ef0c5f4c0515a98e55c43dbe4)
+++ core/Controller/LostController.php	(date 1565692966939)
@@ -194,8 +194,12 @@
 			throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid'));
 		}
 
-		try {
-			$encryptedToken = $this->config->getUserValue($userId, 'core', 'lostpassword', null);
+		$encryptedToken = $this->config->getUserValue($userId, 'core', 'lostpassword', null);
+		if ($encryptedToken === null) {
+			throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid'));
+		}
+
+		try {
 			$mailAddress = !is_null($user->getEMailAddress()) ? $user->getEMailAddress() : '';
 			$decryptedToken = $this->crypto->decrypt($encryptedToken, $mailAddress.$this->config->getSystemValue('secret'));
 		} catch (\Exception $e) {

internal server ip ( that scared me a little)

You should see the remote addr (your client ip) there. If not thats usually a sign that you run nextcloud in a reverse proxy scenario without proper configuration: https://docs.nextcloud.com/server/16/admin_manual/configuration_server/reverse_proxy_configuration.html.

cc @nextcloud/security

[nickvergessen]

No security implication, your patch looks good