fix(security): prevent XSS vulnerability in initialize() checkout modal#96
fix(security): prevent XSS vulnerability in initialize() checkout modal#96Olayiwolaaa wants to merge 6 commits into
Conversation
Signoz monitoring implementation
Pull logs for v1.2.0
|
Hey @Olayiwolaaa, Thanks for flagging this! A couple of things before we can move forward:
Appreciate the contribution! |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 33875719 | Triggered | Generic High Entropy Secret | 6ecf109 | src/Monitoring/SignozServiceLogger.php | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Hi @Abraham-Flutterwave, thanks for the feedback!
Let me know if anything needs adjusting! |
|
Hey @Olayiwolaaa Please update your test with the ModalEventHandler. see the snippet below: use Flutterwave\EventHandlers\ModalEventHandler; # add this line
private function buildInstance(): Flutterwave
{
$instance = new Flutterwave();
$instance
->eventHandler(new ModalEventHandler()) # add this line
->setAmount('1000')
->setCurrency('NGN')
...so in src/Flutterwave.php you forgot to use the public function initialize(): void
{
$checkoutConfig = [
'public_key' => self::$config->getPublicKey(),
'tx_ref' => $this->txref,
'amount' => (float) $this->amount,
'currency' => $this->currency,
'country' => $this->country,
'payment_options' => $this->paymentOptions,
'redirect_url' => $this->redirectUrl,
'first_name' => $this->customerFirstname,
'last_name' => $this->customerLastname,
'email' => $this->customerEmail,
'phone_number' => $this->customerPhone,
'payment_method' => $this->paymentOptions,
'customizations' => [
'title' => $this->customTitle,
'description' => $this->customDescription,
'logo' => $this->customLogo,
],
];
...
echo $this->render(Modal::POPUP)->with($checkoutConfig)->getHtml();
}Please run the command thanks |
- Move customer fields (email, phone_number, first_name, last_name) to top level in checkout config to match Modal::with() contract - Add payment_method key alongside payment_options - Pass customizations through to Modal via payload setters - Add JSON_HEX_APOS and JSON_PRESERVE_ZERO_FRACTION flags to json_encode in Modal::getHtml() for proper escaping and float output - Fix testInitializeEscapesScriptTagInTitle to not match legitimate HTML template script tags - Rewrite testInitializeIsDeprecated to use set_error_handler since @trigger_error suppresses PHPUnit deprecation conversion
|
Hey @Abraham-Flutterwave Changes:
Testing All 6 tests in |
Summary
The
initialize()method was interpolating user-supplied values directlyinto a
<script>block without any escaping, allowing XSS attacks viaany of the customer or customization fields.
Problem
Any field passed to the checkout modal (name, email, title, redirect URL,
etc.) could be used to break out of the JS string context. For example:
setTitle('</script><script>alert(1)</script>')would inject and executearbitrary JavaScript on the payment page.
setRedirectUrl('https://evil.com","public_key":"leaked')could leakthe public key or redirect users to a phishing page.
Changes
Commit 1 — Quick patch: wrapped all interpolated string values with
addslashes()and cast$this->amountto(float)to close theimmediate injection surfaces.
Commit 2 — Proper fix: replaced the entire string-concatenated JS
block with
json_encode()usingJSON_HEX_TAG | JSON_HEX_QUOT | JSON_THROW_ON_ERROR, which handles all escaping correctly including</script>injection thataddslashes()does not cover.Also wired up
$this->paymentOptionswhich was set viasetPaymentOptions()but never actually used in the output — it was hardcoded before.
Verification
Manually verified by setting:
Page source confirmed the payload was safely encoded as:
No alert fired. Modal loaded normally.
Testing
tests/Unit/Checkout/InitializeTest.phpcovers XSS payloads intitle, customer name, redirect URL, amount casting, and payment options
examples/endpoint/test-initialize.phpNotes
setup.phpwherefile_exists('.env')uses cwd instead of
__DIR__, causing.envnot to be found in someenvironments. Submitted as a separate PR: fix: resolve .env loading failure when running under PHP-FPM #97 .