diff --git a/src/CoreShop/Bundle/CoreBundle/Resources/install/pimcore/classes/CoreShopUserBundle/CoreShopUser.json b/src/CoreShop/Bundle/CoreBundle/Resources/install/pimcore/classes/CoreShopUserBundle/CoreShopUser.json index d3541ede8d..7684f67263 100644 --- a/src/CoreShop/Bundle/CoreBundle/Resources/install/pimcore/classes/CoreShopUserBundle/CoreShopUser.json +++ b/src/CoreShop/Bundle/CoreBundle/Resources/install/pimcore/classes/CoreShopUserBundle/CoreShopUser.json @@ -128,6 +128,28 @@ "visibleGridView": false, "visibleSearch": false }, + { + "fieldtype": "datetime", + "queryColumnType": "bigint", + "columnType": "bigint", + "phpdocType": "\\Carbon\\Carbon", + "defaultValue": null, + "useCurrentDate": false, + "name": "passwordResetHashCreatedAt", + "title": "Reset Password Hash Created At", + "tooltip": "", + "mandatory": false, + "noteditable": true, + "index": false, + "locked": false, + "style": "", + "permissions": null, + "datatype": "data", + "relationType": false, + "invisible": true, + "visibleGridView": false, + "visibleSearch": false + }, { "fieldtype": "coreShopRelation", "stack": "coreshop.customer", diff --git a/src/CoreShop/Bundle/FrontendBundle/Controller/RegisterController.php b/src/CoreShop/Bundle/FrontendBundle/Controller/RegisterController.php index 3db01284a2..d2babcccad 100644 --- a/src/CoreShop/Bundle/FrontendBundle/Controller/RegisterController.php +++ b/src/CoreShop/Bundle/FrontendBundle/Controller/RegisterController.php @@ -28,7 +28,7 @@ use CoreShop\Component\Customer\Context\CustomerContextInterface; use CoreShop\Component\Locale\Context\LocaleContextInterface; use CoreShop\Component\Resource\Factory\FactoryInterface; -use CoreShop\Component\Resource\Repository\RepositoryInterface; +use CoreShop\Component\User\Repository\UserRepositoryInterface; use Symfony\Component\DependencyInjection\Attribute\Autowire; use Symfony\Component\EventDispatcher\GenericEvent; use Symfony\Component\HttpFoundation\Request; @@ -81,20 +81,32 @@ public function passwordResetRequestAction(Request $request): Response if ($handledForm->isSubmitted() && $handledForm->isValid()) { $passwordResetData = $handledForm->getData(); - $user = $this->container->get('coreshop.repository.user')->findByLoginIdentifier($passwordResetData['email']); + /** @var UserRepositoryInterface $userRepository */ + $userRepository = $this->container->get('coreshop.repository.user'); + $user = $userRepository->findByLoginIdentifier($passwordResetData['email']); - if (!$user instanceof UserInterface) { - return $this->redirectToRoute('coreshop_index'); - } + // Process the request only if user exists, but always show the same response + // to prevent user enumeration (CWE-204) + if ($user instanceof UserInterface) { + // Generate cryptographically secure token (CWE-330, CWE-338, CWE-640) + $rawToken = $this->generateSecureResetToken(); + + // Store only the hash of the token (CWE-640) + $tokenHash = hash('sha256', $rawToken); + $user->setPasswordResetHash($tokenHash); - $user->setPasswordResetHash($this->generateResetPasswordHash($user)); - $user->save(); + // Set token creation time for TTL enforcement (CWE-613) + $user->setPasswordResetHashCreatedAt(new \DateTimeImmutable()); + $user->save(); - $resetLink = $this->generateUrl('coreshop_customer_password_reset', ['token' => $user->getPasswordResetHash()], UrlGeneratorInterface::ABSOLUTE_URL); + // Use the raw token in the reset link (not the hash) + $resetLink = $this->generateUrl('coreshop_customer_password_reset', ['token' => $rawToken], UrlGeneratorInterface::ABSOLUTE_URL); - $dispatcher = $this->container->get('event_dispatcher'); - $dispatcher->dispatch(new RequestPasswordChangeEvent($user, $resetLink), 'coreshop.user.request_password_reset'); + $dispatcher = $this->container->get('event_dispatcher'); + $dispatcher->dispatch(new RequestPasswordChangeEvent($user, $resetLink), 'coreshop.user.request_password_reset'); + } + // Always show success message regardless of user existence (CWE-204) $this->addFlash('success', $this->container->get('translator')->trans('coreshop.ui.password_reset_request_success')); return $this->redirectToRoute('coreshop_login'); @@ -111,10 +123,12 @@ public function passwordResetAction(Request $request): Response $resetToken = $this->getParameterFromRequest($request, 'token'); if ($resetToken) { - /** - * @var UserInterface $user - */ - $user = $this->container->get('coreshop.repository.user')->findByResetToken($resetToken); + /** @var UserRepositoryInterface $userRepository */ + $userRepository = $this->container->get('coreshop.repository.user'); + + // Use secure token validation with TTL (default 1 hour) + // This validates the hashed token and checks expiration (CWE-613, CWE-640) + $user = $userRepository->findByResetTokenSecure($resetToken); if (!$user) { throw new NotFoundHttpException(); @@ -128,7 +142,9 @@ public function passwordResetAction(Request $request): Response if ($handledForm->isSubmitted() && $handledForm->isValid()) { $resetPassword = $handledForm->getData(); + // Clear the reset token and timestamp (single-use token) $user->setPasswordResetHash(null); + $user->setPasswordResetHashCreatedAt(null); $user->setPassword($resetPassword['password']); $user->save(); @@ -155,7 +171,7 @@ public static function getSubscribedServices(): array parent::getSubscribedServices(), [ new SubscribedService('coreshop.factory.customer', FactoryInterface::class, attributes: new Autowire(service: 'coreshop.factory.customer')), - new SubscribedService('coreshop.repository.user', RepositoryInterface::class, attributes: new Autowire(service: 'coreshop.repository.user')), + new SubscribedService('coreshop.repository.user', UserRepositoryInterface::class, attributes: new Autowire(service: 'coreshop.repository.user')), new SubscribedService('coreshop.context.locale', LocaleContextInterface::class), new SubscribedService(CustomerManagerInterface::class, CustomerManagerInterface::class), new SubscribedService('event_dispatcher', EventDispatcherInterface::class), @@ -178,10 +194,28 @@ protected function getCustomer(): ?CustomerInterface return null; } + /** + * Generate a cryptographically secure reset token using random_bytes(). + * This replaces the insecure MD5+mt_rand approach (CWE-330, CWE-338, CWE-640). + */ + protected function generateSecureResetToken(): string + { + return bin2hex(random_bytes(32)); + } + + /** + * @deprecated Use generateSecureResetToken() instead. This method is insecure and will be removed. + * + * @throws \RuntimeException Always throws to prevent accidental insecure usage + */ protected function generateResetPasswordHash(UserInterface $customer): string { - $this->getParameter('coreshop.customer.security.login_identifier'); + trigger_error( + 'generateResetPasswordHash() is deprecated and insecure. Use generateSecureResetToken() instead.', + \E_USER_DEPRECATED, + ); - return hash('md5', $customer->getId() . $customer->getLoginIdentifier() . mt_rand() . time()); + // Redirect to secure implementation to prevent accidental insecure token generation + return $this->generateSecureResetToken(); } } diff --git a/src/CoreShop/Bundle/UserBundle/Pimcore/Repository/UserRepository.php b/src/CoreShop/Bundle/UserBundle/Pimcore/Repository/UserRepository.php index ff28b1d09d..052f3f4474 100644 --- a/src/CoreShop/Bundle/UserBundle/Pimcore/Repository/UserRepository.php +++ b/src/CoreShop/Bundle/UserBundle/Pimcore/Repository/UserRepository.php @@ -24,6 +24,9 @@ class UserRepository extends PimcoreRepository implements UserRepositoryInterface { + /** + * @deprecated Use findByResetTokenSecure() instead for proper token validation with hashing + */ public function findByResetToken(string $resetToken): ?UserInterface { $list = $this->getList(); @@ -37,6 +40,51 @@ public function findByResetToken(string $resetToken): ?UserInterface return null; } + public function findByResetTokenSecure(string $resetToken, int $ttlSeconds = 3600): ?UserInterface + { + // Validate TTL is positive + if ($ttlSeconds <= 0) { + throw new \InvalidArgumentException('TTL must be a positive integer'); + } + + // Validate token format (expected: 64 hexadecimal characters from bin2hex(random_bytes(32))) + if (strlen($resetToken) !== 64 || !ctype_xdigit($resetToken)) { + return null; + } + + // Hash the provided token to compare against stored hash + $tokenHash = hash('sha256', $resetToken); + + $list = $this->getList(); + $list->setCondition('passwordResetHash = ?', [$tokenHash]); + $objects = $list->load(); + + if (count($objects) !== 1 || !$objects[0] instanceof UserInterface) { + return null; + } + + $user = $objects[0]; + + // Check token expiration (TTL) + $createdAt = $user->getPasswordResetHashCreatedAt(); + if ($createdAt === null) { + return null; + } + + // Convert to immutable to prevent side effects when calling modify() + $createdAtImmutable = \DateTimeImmutable::createFromInterface($createdAt); + + $now = new \DateTimeImmutable(); + $expiresAt = $createdAtImmutable->modify('+' . $ttlSeconds . ' seconds'); + + if ($now > $expiresAt) { + // Token has expired + return null; + } + + return $user; + } + public function findByLoginIdentifier(string $value): ?UserInterface { $list = $this->getList(); diff --git a/src/CoreShop/Bundle/UserBundle/Resources/install/pimcore/classes/CoreShopUser.json b/src/CoreShop/Bundle/UserBundle/Resources/install/pimcore/classes/CoreShopUser.json index 8015d20f5d..34705ee148 100644 --- a/src/CoreShop/Bundle/UserBundle/Resources/install/pimcore/classes/CoreShopUser.json +++ b/src/CoreShop/Bundle/UserBundle/Resources/install/pimcore/classes/CoreShopUser.json @@ -125,6 +125,28 @@ "invisible": true, "visibleGridView": false, "visibleSearch": false + }, + { + "fieldtype": "datetime", + "queryColumnType": "bigint", + "columnType": "bigint", + "phpdocType": "\\Carbon\\Carbon", + "defaultValue": null, + "useCurrentDate": false, + "name": "passwordResetHashCreatedAt", + "title": "Reset Password Hash Created At", + "tooltip": "", + "mandatory": false, + "noteditable": true, + "index": false, + "locked": false, + "style": "", + "permissions": null, + "datatype": "data", + "relationType": false, + "invisible": true, + "visibleGridView": false, + "visibleSearch": false } ], "locked": false diff --git a/src/CoreShop/Component/User/Model/UserInterface.php b/src/CoreShop/Component/User/Model/UserInterface.php index 6c3879f2af..305b7b7836 100644 --- a/src/CoreShop/Component/User/Model/UserInterface.php +++ b/src/CoreShop/Component/User/Model/UserInterface.php @@ -43,4 +43,8 @@ public function getPlainPassword(): ?string; public function getPasswordResetHash(): ?string; public function setPasswordResetHash(?string $passwordResetHash); + + public function getPasswordResetHashCreatedAt(): ?\DateTimeInterface; + + public function setPasswordResetHashCreatedAt(?\DateTimeInterface $passwordResetHashCreatedAt); } diff --git a/src/CoreShop/Component/User/Repository/UserRepositoryInterface.php b/src/CoreShop/Component/User/Repository/UserRepositoryInterface.php index 199df271ca..867639de1a 100644 --- a/src/CoreShop/Component/User/Repository/UserRepositoryInterface.php +++ b/src/CoreShop/Component/User/Repository/UserRepositoryInterface.php @@ -23,7 +23,20 @@ interface UserRepositoryInterface extends PimcoreRepositoryInterface { + /** + * @deprecated Use findByResetTokenSecure() instead for proper token validation with hashing + */ public function findByResetToken(string $resetToken): ?UserInterface; + /** + * Find a user by validating the reset token against the stored hash. + * + * @param string $resetToken The raw token provided by the user + * @param int $ttlSeconds The maximum age of the token in seconds (default: 3600 = 1 hour) + * + * @return UserInterface|null The user if token is valid and not expired, null otherwise + */ + public function findByResetTokenSecure(string $resetToken, int $ttlSeconds = 3600): ?UserInterface; + public function findByLoginIdentifier(string $value): ?UserInterface; }