An empty webhook secret in the Svix PHP library, and a fix that was merged the same day

The problem

Svix signs every webhook with HMAC-SHA256. The secret a merchant copies from the dashboard looks like whsec_ followed by base64, and the library's Webhook class turns it into the HMAC key. In PHP the constructor did this:

if (substr($secret, 0, strlen(Webhook::SECRET_PREFIX)) === Webhook::SECRET_PREFIX) {
    $secret = substr($secret, strlen(Webhook::SECRET_PREFIX));
}
$this->secret = base64_decode($secret);

Without its strict flag, base64_decode does not fail on bad input. It skips characters outside the base64 alphabet and returns whatever is left. An empty string, whsec_ alone, or a string with no valid characters such as @@@@ all decode to "". The class then signed and verified with hash_hmac('sha256', ..., ""), an HMAC with an empty key.

Why it matters

An HMAC with an empty key is still a valid HMAC, and anyone can compute it. A verifier in that state accepts any payload signed with no key at all, so an attacker who knows the endpoint can send forged events, and the application treats them as genuine.

The way to get there is ordinary. From the issue:

new Webhook((string) getenv('SVIX_WEBHOOK_SECRET'));

When the variable is not set, getenv returns false, the cast turns it into "", and the application starts without any error. Real webhooks fail verification, which looks like a configuration problem to debug, while forged ones pass.

Svix had already closed this gap in the other libraries: #2347 added an empty secret check to Go, Rust, Ruby, C# and Java. PHP was not part of that change.

The fix

A maintainer replied within two hours that the suggested fix sounded good and asked for a pull request. #2765 decodes in strict mode and refuses an empty result:

$secret = base64_decode($secret, true);
if ($secret === false || $secret === '') {
    throw new \InvalidArgumentException("secret must be a non-empty base64 string");
}
$this->secret = $secret;

InvalidArgumentException is what the constructor already throws for a bad tolerance, so callers do not need a new exception type. fromRaw(), which takes the raw key bytes instead of the whsec_ form, now rejects an empty secret too.

The tests cover "", "whsec_", "whsec_@@@@" and fromRaw(""). They fail without the change and pass with it on PHP 8.4. The pull request also adds a ChangeLog entry, because a secret that silently worked before now throws at startup, and that is a behavior change users should see in the release notes.

The follow-up: fromRaw never worked

While writing the fix I noticed that fromRaw() itself was broken: it calls new self(), but the constructor requires a secret, so every call throws ArgumentCountError. An earlier pull request, #2437, was already fixing that and had review feedback, so #2765 left it alone. When the maintainer approved #2765, they noted that #2437 had gone quiet and asked whether I would pick it up.

#2768 does that with the feedback from #2437. The constructor takes the raw secret as an optional third parameter and requires exactly one of the two forms, so new Webhook() with no secret still throws instead of returning an object that cannot verify anything. fromRaw() gains an optional tolerance as well. The tests cover verifying through fromRaw() with the default and a custom tolerance, a wrong raw secret, no secret and both secrets.

Takeaways

• A lenient decoder turns a missing secret into a weak one. Use the strict mode and check the result, or validate the format before decoding.
• Reject an empty key where the key enters the library, not where it is used. That is the one place every code path goes through.
• When a fix lands in most libraries of a family, check the ones it skipped. The same gap shows up in the Adyen libraries, where it led to seven pull requests.