unhandled RuntimeException

[philipptreichel]

path: vendor/web-token/jwt-signature/JWSVerifier.php

private function verifySignature(JWS $jws, JWKSet $jwkset, Signature $signature, ?string $detachedPayload = null, JWK &$successJwk = null): bool
    {
        $input = $this->getInputToVerify($jws, $signature, $detachedPayload);
        $algorithm = $this->getAlgorithm($signature);
        foreach ($jwkset->all() as $jwk) {
            try {
                KeyChecker::checkKeyUsage($jwk, 'verification');
                KeyChecker::checkKeyAlgorithm($jwk, $algorithm->name());
                if (true === $algorithm->verify($jwk, $input, $signature->getSignature())) {
                    $successJwk = $jwk;

                    return true;
                }
            } catch (Throwable $e) {
                //We do nothing, we continue with other keys
                continue;
            }
        }

        return false;
    }

Problem Description

This Function catches all Exceptions. When a Runtime Exception occures such as:

RuntimeException {#120
  #message: "Requires GMP or bcmath extension."
  #code: 0
  #file: "/var/www/html/vendor/fgrosse/phpasn1/lib/Utility/BigInteger.php"
  #line: 62

this function just gives a false back.

Possible solution

Catch Runtime Exceptions Separately and throw a new Exception. Maybe like this:

private function verifySignature(JWS $jws, JWKSet $jwkset, Signature $signature, ?string $detachedPayload = null, JWK &$successJwk = null): bool
    {
        $input = $this->getInputToVerify($jws, $signature, $detachedPayload);
        $algorithm = $this->getAlgorithm($signature);
        foreach ($jwkset->all() as $jwk) {
            try {
                KeyChecker::checkKeyUsage($jwk, 'verification');
                KeyChecker::checkKeyAlgorithm($jwk, $algorithm->name());
                if (true === $algorithm->verify($jwk, $input, $signature->getSignature())) {
                    $successJwk = $jwk;

                    return true;
                }
            } catch (Throwable $e) {
                //We do nothing, we continue with other keys

                continue;
            } catch (\RuntimeException $e) {
              // Throe new exception so that environment errors are noticed
              throw new \RuntimeException($e->getMessage(), $e);
            }
        }

        return false;
    }

[Spomky]

Hi @philipptreichel,

You are right, this could be managed as showed in your proposal. Maybe just throw $e; is enough, no need to wrap the RuntimeException into another RuntimeException.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.