Skip to content

feat(POS-1338): Implement ActionHandler instance binding (remedy cross-contamination) #190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mateusz-walesiak-cko wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat(POS-1338): Implement ActionHandler instance binding (remedy cross-contamination) #190

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions src/processout/actionhandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,16 @@
protected options: ActionHandlerOptions;

/**
* listenerCount is the number of listener that were set
* @type {number}
* Action handler static counter to track events
* @var {number}
*/
protected static listenerCount = 0;
protected static listenerCount: number = 0;

/**
* Unique identifier for this ActionHandler instance
* @var {string}
*/
protected instanceUID: string;

/**
* newWindowName is the name of the new windows created by the
Expand All @@ -217,6 +223,11 @@

if (!this.options) this.options = new ActionHandlerOptions();

// Generate unique identifier for this instance to prevent cross-contamination
this.instanceUID = `action_${Math.random().toString(36).substring(7)}`;

Check failure

Code scanning / CodeQL

Insecure randomness High

This uses a cryptographically insecure random number generated at
Math.random()
Loading
in a security context.

Copilot Autofix

AI 7 months ago

To fix the issue, replace the use of Math.random() with a cryptographically secure random number generator. In Node.js, the crypto module provides the randomBytes method, which can be used to generate secure random values. The fix involves:

  1. Importing the crypto module.
  2. Using crypto.randomBytes to generate a secure random value.
  3. Converting the random bytes into a string format similar to the original implementation.

The replacement ensures that the instanceUID is generated securely while maintaining the original functionality.

Suggested changeset 1
src/processout/actionhandler.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/processout/actionhandler.ts b/src/processout/actionhandler.ts
--- a/src/processout/actionhandler.ts
+++ b/src/processout/actionhandler.ts
@@ -1,2 +1,3 @@
 /// <reference path="../references.ts" />
+import * as crypto from 'crypto';
 
@@ -226,3 +227,4 @@
             // Generate unique identifier for this instance to prevent cross-contamination
-            this.instanceUID = `action_${Math.random().toString(36).substring(7)}`;
+            const randomBytes = crypto.randomBytes(16); // Generate 16 random bytes
+            this.instanceUID = `action_${randomBytes.toString('hex').substring(0,7)}`;
             
EOF
@@ -1,2 +1,3 @@
/// <reference path="../references.ts" />
import * as crypto from 'crypto';

@@ -226,3 +227,4 @@
// Generate unique identifier for this instance to prevent cross-contamination
this.instanceUID = `action_${Math.random().toString(36).substring(7)}`;
const randomBytes = crypto.randomBytes(16); // Generate 16 random bytes
this.instanceUID = `action_${randomBytes.toString('hex').substring(0,7)}`;

Copilot is powered by AI and may make mistakes. Always verify output.

// Note: setResourceID() should be invoked when calling specific action methods

// We need to create the wrapper beforehand
if (this.options.flow == ActionFlow.IFrame) {
var iframeWrapper = document.createElement("div");
Expand Down Expand Up @@ -466,6 +477,11 @@
if (data.namespace != Message.checkoutNamespace)
return;

// Validate the message is intended for this specific ActionHandler instance to prevent cross-contamination
if (data.frameID && data.frameID !== this.instanceUID) {
return; // Message is for a different ActionHandler instance
}

// Not the latest listener anymore
if (ActionHandler.listenerCount != cur) {
// Reset the timer if it hasn't been done already
Expand Down Expand Up @@ -551,6 +567,24 @@
return this.canceled;
}

/**
* Set the resource ID for this ActionHandler instance
* @param {string} resourceID
* @return {void}
*/
public setResourceID(resourceID: string): void {
this.resourceID = resourceID;
}

/**
* Get the unique instance ID for this ActionHandler
* This can be used by checkout pages to include in postMessage responses
* @return {string}
*/
public getInstanceUID(): string {
return this.instanceUID;
}

}

}
Loading