diff --git a/CODE_AUDIT.md b/CODE_AUDIT.md new file mode 100644 index 00000000..fff7939d --- /dev/null +++ b/CODE_AUDIT.md @@ -0,0 +1,182 @@ +# ENgrid Code Audit + +**Date:** 2026-02-11 +**Scope:** `packages/scripts/src/` (~15,700 lines TypeScript) +**Approach:** Focused on real bugs that cause incorrect behavior in practice, versus things that could theoretically be true at the extreme. Good code is called out so you know an area was reviewed. + +--- + +## Real Bugs Found & Fixed + +### BUG-1: `onLoad` handler bound but never called +**File:** `app.ts:501` + +`.bind()` returns a new function but doesn't invoke it. Any pre-existing `window.onload` handler was silently dropped. + +**Fix:** Changed to `onLoad.call(window, e)`. + +--- + +### BUG-2/3: Null crash in `DonationAmount.clearOther()` and `load()` +**File:** `events/donation-amount.ts:82-87, 167-173` + +Both methods query for the "other amount" input and immediately access `.value` without a null check. The `as HTMLInputElement` cast silences TypeScript but doesn't prevent the runtime TypeError. On pages where the client removes the "other" field, this crashes. + +**Fix:** Added null guards before accessing the element. + +--- + +### BUG-4: `splice()` during `forEach` skips elements +**File:** `show-hide-radio-checkboxes.ts:141-146, 161-166` + +When removing stale state entries, `splice()` inside `forEach` mutates the array mid-iteration. If two consecutive entries share the same class, splicing the first shifts the second into its index position, and `forEach` advances past it. The duplicate remains in sessionStorage. + +**Fix:** Replaced with `filter()` to build a clean array, then repopulated in place. + +--- + +### BUG-5: `_dispatch` flag permanently stuck after error +**File:** `events/donation-amount.ts:114-164`, `events/donation-frequency.ts:108-145` + +`setAmount()` and `setFrequency()` set `this._dispatch = dispatch` at the start and `this._dispatch = true` at the end. If any operation between them throws (e.g., from BUG-2/3's null access), `_dispatch` stays `false` permanently. All subsequent amount/frequency changes silently stop dispatching events, breaking live variables, submit button labels, processing fees, and upsell logic. + +**Fix:** Wrapped operation bodies in `try/finally` to always restore `_dispatch = true`. + +--- + +### BUG-6: Exit intent fires `open()` twice per trigger +**File:** `exit-intent-lightbox.ts:86-99` + +When `relatedTarget` is null (the real trigger), `open()` was called immediately AND a timeout was set to call it again. The `this.opened` guard prevents visible duplication, but it still pushes a duplicate `exit_intent_lightbox_shown` event to the dataLayer. + +**Fix:** Removed the immediate call; the timeout alone handles it. + +--- + +### BUG-7: `JSON.parse` without try/catch in RememberMe +**File:** `remember-me.ts:151-153` + +`updateFieldData()` parses cookie/postMessage data with bare `JSON.parse()`. If the cookie is corrupted (partial write, encoding issue, manual edit), this throws an unhandled exception. The iframe message handler at line 113 correctly uses `isJson()` first, but the cookie path at line 348 passes data directly. + +**Fix:** Wrapped in try/catch; malformed data is silently ignored so the form loads fresh. + +--- + +### BUG-8: `postMessage` with wildcard `"*"` origin +**File:** `remember-me.ts:94-96, 336-343`, `iframe.ts:93, 162-167, 185-193` + +Outgoing `postMessage` calls used `"*"` as the target origin. Since ENgrid forms are designed to be embedded in iframes on third-party sites, any embedding page could receive these messages. RememberMe messages contain supporter field data. + +Note: the incoming direction was already correct -- `remember-me.ts:318` verifies `event.source === iframe.contentWindow`. + +**Fix:** +- RememberMe: Extract origin from the configured `remoteUrl` via `new URL().origin`. +- iFrame: Derive parent origin from `document.referrer`, falling back to `"*"` only when the referrer is unavailable. + +--- + +### BUG-9: `sessionStorage` access without try/catch in DonationFrequency +**File:** `events/donation-frequency.ts:36, 63-66, 86` + +Firefox with cookies disabled, some Safari private browsing modes, and various privacy extensions throw on `sessionStorage` access. The `App` class already wraps its RememberMe and DebugPanel sessionStorage in try/catch (lines 353-364, 471-479), showing the team is aware of this issue, but `DonationFrequency` was missed. + +**Fix:** Wrapped all three sessionStorage access points in try/catch. + +--- + +## Moderate Patterns Fixed + +### MOD-1: `Function()` constructor for upsell calculation +**File:** `upsell-lightbox.ts:247-249` + +Used `Function('"use strict";return (' + expression + ')')()` to evaluate expressions like `"amount / 12"`. The input is developer-configured (not user form input), so the security risk is limited to the client's own page config. However, `Function()` is flagged by CSP policies and code scanners. + +**Fix:** Replaced with a safe arithmetic evaluator that validates input against `[0-9\s.+\-*/()]` and uses a reduce-based parser. + +### MOD-3: Infinite `run()` retry loop +**File:** `app.ts:157-161` + +If the EN framework never loads, `run()` retried every 100ms forever. In practice this rarely matters on EN-hosted pages, but on misconfigured pages it creates a silent infinite loop. + +**Fix:** Added a retry counter, stops after 50 attempts (~5 seconds) with a clear error message. + +--- + +## Moderate Issues NOT Fixed (Correct but worth noting) + +### MOD-2: `innerHTML` with option-derived content +**Files:** `upsell-lightbox.ts`, `exit-intent-lightbox.ts`, `live-variables.ts`, `engrid.ts` + +Content comes from developer-set options, not user form input. Safe in current usage but would become XSS vectors if content sources change. No action needed now. + +### MOD-4: 1000ms hardcoded timeout for frequency loading +**File:** `app.ts:319-321` + +The comment acknowledges a ~20% race condition with SwapAmounts. A MutationObserver would be more reliable, but the timeout works well enough for the vast majority of page loads. + +### MOD-5: `==` instead of `===` throughout +All flagged instances compare strings to string literals -- `==` and `===` behave identically. No bugs result. A codebase-wide cleanup to `===` would improve consistency but isn't a correctness issue. + +### MOD-6: NeverBounce array access without bounds check +**File:** `neverbounce.ts:64-65` + +Runs inside a `setTimeout` after NB registration when the field should exist. Wrapping in a null check would be defensive but the current code works in practice. + +--- + +## Good Code (Areas reviewed and solid) + +### Singleton event system +**Files:** `events/donation-amount.ts`, `events/donation-frequency.ts`, `events/en-form.ts`, `events/processing-fees.ts` + +The `getInstance()` pattern with strongly-typed-events gives type-safe pub/sub with exactly one event bus per concern. Solid architecture that keeps modules decoupled. + +### `checkNested()` for deep property access +**File:** `engrid.ts:445-453` + +Used consistently before accessing deeply nested EN framework properties. Prevents cascading TypeErrors from the third-party framework not being in the expected state. + +### RememberMe incoming message validation +**File:** `remember-me.ts:318` + +Correctly verifies `event.source === iframe.contentWindow` to prevent other windows from injecting data. + +### `cleanAmount()` currency parsing +**File:** `engrid.ts:358-396` + +Handles international currency formats (comma vs dot separators, varying decimal places). Correctly handles `1,234.56`, `1.234,56`, `1234`, and returns 0 for ambiguous inputs. + +### `watchForError` callback deduplication +**File:** `engrid.ts:574-604` + +Uses data attributes on the error element to prevent registering the same MutationObserver callback twice. Creative and effective. + +### Try/catch around RememberMe and DebugPanel sessionStorage +**File:** `app.ts:353-364, 471-479` + +Correctly handles browsers where localStorage/sessionStorage throws. + +### Digital wallets MutationObserver with disconnect +**File:** `digital-wallets.ts:173-196` + +Observer disconnects itself after detecting wallet buttons, preventing it from running indefinitely. Right pattern for one-time DOM detection. + +### DataLayer PII handling +**File:** `data-layer.ts:26-60, 330-338` + +Sensitive fields (CC numbers, bank accounts) are excluded. PII fields use `crypto.subtle.digest("SHA-256")` (the comment at line 8 mentioning Base64/btoa is outdated but the implementation is correct). + +### VGS expiration date validation +**File:** `vgs.ts:218-276` + +Month/year cross-validation correctly prevents selecting expired dates by disabling past months when current year is selected. + +### Modal focus trap +**File:** `modal.ts:118-149` + +Correct Tab/Shift+Tab focus wrapping. The handler is an arrow function property so `addEventListener`/`removeEventListener` correctly pair up. + +### Modular feature initialization +**File:** `app.ts:185-467` + +Features conditionally initialized based on options and page type. Each feature class guards its own constructor. Clean and prevents cross-feature interference. diff --git a/packages/scripts/dist/app.d.ts b/packages/scripts/dist/app.d.ts index 4e59c400..84813d24 100644 --- a/packages/scripts/dist/app.d.ts +++ b/packages/scripts/dist/app.d.ts @@ -7,6 +7,7 @@ export declare class App extends ENGrid { private _country; private _dataLayer; private options; + private _runRetries; private logger; constructor(options: Options); private run; diff --git a/packages/scripts/dist/app.js b/packages/scripts/dist/app.js index 7b7d433e..d3235482 100644 --- a/packages/scripts/dist/app.js +++ b/packages/scripts/dist/app.js @@ -9,6 +9,7 @@ export class App extends ENGrid { this._amount = DonationAmount.getInstance("transaction.donationAmt", "transaction.donationAmt.other"); this._frequency = DonationFrequency.getInstance(); this._country = Country.getInstance(); + this._runRetries = 0; this.logger = new EngridLogger("App", "black", "white", "🍏"); const loader = new Loader(); this.options = Object.assign(Object.assign({}, OptionsDefaults), options); @@ -45,6 +46,11 @@ export class App extends ENGrid { } run() { if (!ENGrid.checkNested(window.EngagingNetworks, "require", "_defined", "enjs")) { + this._runRetries++; + if (this._runRetries > 50) { + this.logger.danger("Engaging Networks JS Framework NOT FOUND after 50 retries. Giving up."); + return; + } this.logger.danger("Engaging Networks JS Framework NOT FOUND"); setTimeout(() => { this.run(); @@ -309,7 +315,7 @@ export class App extends ENGrid { window.onload = (e) => { this.onLoad(); if (onLoad) { - onLoad.bind(window, e); + onLoad.call(window, e); } }; } diff --git a/packages/scripts/dist/events/donation-amount.js b/packages/scripts/dist/events/donation-amount.js index 2d544cc7..4733202c 100644 --- a/packages/scripts/dist/events/donation-amount.js +++ b/packages/scripts/dist/events/donation-amount.js @@ -64,8 +64,10 @@ export class DonationAmount { } else { const otherField = document.querySelector('input[name="' + this._other + '"]'); - currentAmountValue = ENGrid.cleanAmount(otherField.value); - this.amount = currentAmountValue; + if (otherField) { + currentAmountValue = ENGrid.cleanAmount(otherField.value); + this.amount = currentAmountValue; + } } } else if (ENGrid.checkNested(window.EngagingNetworks, "require", "_defined", "enjs", "getDonationTotal") && @@ -85,49 +87,57 @@ export class DonationAmount { } // Set dispatch to be checked by the SET method this._dispatch = dispatch; - // Search for the current amount on radio boxes - let found = Array.from(document.querySelectorAll('input[name="' + this._radios + '"]')).filter((el) => el instanceof HTMLInputElement && parseInt(el.value) == amount); - // We found the amount on the radio boxes, so check it - if (found.length) { - const amountField = found[0]; - amountField.checked = true; - // Change Event - const event = new Event("change", { - bubbles: true, - cancelable: true, - }); - amountField.dispatchEvent(event); - // Clear OTHER text field - this.clearOther(); - } - else { - const otherField = document.querySelector('input[name="' + this._other + '"]'); - if (otherField) { - const enFieldOtherAmountRadio = document.querySelector(`.en__field--donationAmt.en__field--withOther .en__field__item:nth-last-child(2) input[name="${this._radios}"]`); - if (enFieldOtherAmountRadio) { - enFieldOtherAmountRadio.checked = true; - } - otherField.value = parseFloat(amount.toString()).toFixed(2); + try { + // Search for the current amount on radio boxes + let found = Array.from(document.querySelectorAll('input[name="' + this._radios + '"]')).filter((el) => el instanceof HTMLInputElement && parseInt(el.value) == amount); + // We found the amount on the radio boxes, so check it + if (found.length) { + const amountField = found[0]; + amountField.checked = true; // Change Event const event = new Event("change", { bubbles: true, cancelable: true, }); - otherField.dispatchEvent(event); - const otherWrapper = otherField.parentNode; - otherWrapper.classList.remove("en__field__item--hidden"); + amountField.dispatchEvent(event); + // Clear OTHER text field + this.clearOther(); + } + else { + const otherField = document.querySelector('input[name="' + this._other + '"]'); + if (otherField) { + const enFieldOtherAmountRadio = document.querySelector(`.en__field--donationAmt.en__field--withOther .en__field__item:nth-last-child(2) input[name="${this._radios}"]`); + if (enFieldOtherAmountRadio) { + enFieldOtherAmountRadio.checked = true; + } + otherField.value = parseFloat(amount.toString()).toFixed(2); + // Change Event + const event = new Event("change", { + bubbles: true, + cancelable: true, + }); + otherField.dispatchEvent(event); + const otherWrapper = otherField.parentNode; + otherWrapper.classList.remove("en__field__item--hidden"); + } } + // Set the new amount and trigger all live variables + this.amount = amount; + } + finally { + // Revert dispatch to default value (true) + this._dispatch = true; } - // Set the new amount and trigger all live variables - this.amount = amount; - // Revert dispatch to default value (true) - this._dispatch = true; } // Clear Other Field clearOther() { const otherField = document.querySelector('input[name="' + this._other + '"]'); + if (!otherField) + return; otherField.value = ""; const otherWrapper = otherField.parentNode; - otherWrapper.classList.add("en__field__item--hidden"); + if (otherWrapper) { + otherWrapper.classList.add("en__field__item--hidden"); + } } } diff --git a/packages/scripts/dist/events/donation-frequency.js b/packages/scripts/dist/events/donation-frequency.js index f3aa1491..7e8da596 100644 --- a/packages/scripts/dist/events/donation-frequency.js +++ b/packages/scripts/dist/events/donation-frequency.js @@ -25,8 +25,14 @@ export class DonationFrequency { }); //Thank you page handling for utility classes if (ENGrid.getGiftProcess()) { - ENGrid.setBodyData("transaction-recurring-frequency", sessionStorage.getItem("engrid-transaction-recurring-frequency") || - "onetime"); + let storedFrequency = "onetime"; + try { + storedFrequency = + sessionStorage.getItem("engrid-transaction-recurring-frequency") || + "onetime"; + } + catch (e) { } + ENGrid.setBodyData("transaction-recurring-frequency", storedFrequency); ENGrid.setBodyData("transaction-recurring", window.pageJson.recurring ? "y" : "n"); } } @@ -45,7 +51,10 @@ export class DonationFrequency { if (this._dispatch) this._onFrequencyChange.dispatch(this._frequency); ENGrid.setBodyData("transaction-recurring-frequency", this._frequency); - sessionStorage.setItem("engrid-transaction-recurring-frequency", this._frequency); + try { + sessionStorage.setItem("engrid-transaction-recurring-frequency", this._frequency); + } + catch (e) { } } get recurring() { return this._recurring; @@ -60,9 +69,15 @@ export class DonationFrequency { // Set amount var with currently selected amount load() { var _a; + let storedFrequency = ""; + try { + storedFrequency = + sessionStorage.getItem("engrid-transaction-recurring-frequency") || ""; + } + catch (e) { } this.frequency = ENGrid.getFieldValue("transaction.recurrfreq") || - sessionStorage.getItem("engrid-transaction-recurring-frequency") || + storedFrequency || "onetime"; const recurrField = ENGrid.getField("transaction.recurrpay"); if (recurrField) { @@ -83,9 +98,13 @@ export class DonationFrequency { } // Set dispatch to be checked by the SET method this._dispatch = dispatch; - ENGrid.setFieldValue("transaction.recurrpay", recurr.toUpperCase()); - // Revert dispatch to default value (true) - this._dispatch = true; + try { + ENGrid.setFieldValue("transaction.recurrpay", recurr.toUpperCase()); + } + finally { + // Revert dispatch to default value (true) + this._dispatch = true; + } } // Force a new frequency setFrequency(freq, dispatch = true) { @@ -95,21 +114,25 @@ export class DonationFrequency { } // Set dispatch to be checked by the SET method this._dispatch = dispatch; - // Search for the current amount on radio boxes - let found = Array.from(document.querySelectorAll('input[name="transaction.recurrfreq"]')).filter((el) => el instanceof HTMLInputElement && el.value == freq.toUpperCase()); - // We found the amount on the radio boxes, so check it - if (found.length) { - const freqField = found[0]; - freqField.checked = true; - this.frequency = freq.toLowerCase(); - if (this.frequency === "onetime") { - this.setRecurrency("N", dispatch); - } - else { - this.setRecurrency("Y", dispatch); + try { + // Search for the current amount on radio boxes + let found = Array.from(document.querySelectorAll('input[name="transaction.recurrfreq"]')).filter((el) => el instanceof HTMLInputElement && el.value == freq.toUpperCase()); + // We found the amount on the radio boxes, so check it + if (found.length) { + const freqField = found[0]; + freqField.checked = true; + this.frequency = freq.toLowerCase(); + if (this.frequency === "onetime") { + this.setRecurrency("N", dispatch); + } + else { + this.setRecurrency("Y", dispatch); + } } } - // Revert dispatch to default value (true) - this._dispatch = true; + finally { + // Revert dispatch to default value (true) + this._dispatch = true; + } } } diff --git a/packages/scripts/dist/exit-intent-lightbox.js b/packages/scripts/dist/exit-intent-lightbox.js index 0a6384ef..18168edf 100644 --- a/packages/scripts/dist/exit-intent-lightbox.js +++ b/packages/scripts/dist/exit-intent-lightbox.js @@ -53,16 +53,10 @@ export class ExitIntentLightbox { // Reliable, works on mouse exiting window and // user switching active program const from = e.relatedTarget; - if (!from) { - this.logger.log("Triggered by mouse position"); - this.open(); - } - if (!this.triggerTimeout) { + if (!from && !this.triggerTimeout) { this.triggerTimeout = window.setTimeout(() => { - if (!from) { - this.logger.log("Triggered by mouse position"); - this.open(); - } + this.logger.log("Triggered by mouse position"); + this.open(); this.triggerTimeout = null; }, this.triggerDelay); } diff --git a/packages/scripts/dist/iframe.d.ts b/packages/scripts/dist/iframe.d.ts index f345457c..7fc316f9 100644 --- a/packages/scripts/dist/iframe.d.ts +++ b/packages/scripts/dist/iframe.d.ts @@ -2,6 +2,7 @@ import { EnForm } from "./events"; export declare class iFrame { _form: EnForm; private logger; + private parentOrigin; constructor(); private onLoaded; private sendIframeHeight; diff --git a/packages/scripts/dist/iframe.js b/packages/scripts/dist/iframe.js index 67ad0120..a1383dc3 100644 --- a/packages/scripts/dist/iframe.js +++ b/packages/scripts/dist/iframe.js @@ -4,6 +4,15 @@ export class iFrame { constructor() { this._form = EnForm.getInstance(); this.logger = new EngridLogger("iFrame", "brown", "gray", "📡"); + // Derive parent origin from referrer for postMessage security + try { + this.parentOrigin = document.referrer + ? new URL(document.referrer).origin + : "*"; + } + catch (e) { + this.parentOrigin = "*"; + } if (this.inIframe()) { // Add the data-engrid-embedded attribute when inside an iFrame if it wasn't already added by a script in the Page Template ENGrid.setBodyData("embedded", ""); @@ -73,7 +82,7 @@ export class iFrame { ? firstError.getBoundingClientRect().top : 0; this.logger.log(`iFrame Event 'scrollTo' - Position of top of first error ${scrollTo} px`); // check the message is being sent correctly - window.parent.postMessage({ scrollTo }, "*"); + window.parent.postMessage({ scrollTo }, this.parentOrigin); // Send the height of the iFrame window.setTimeout(() => { this.sendIframeHeight(); @@ -136,7 +145,7 @@ export class iFrame { this.sendIframeHeight(); window.parent.postMessage({ scroll: this.shouldScroll(), - }, "*"); + }, this.parentOrigin); // On click fire the resize event document.addEventListener("click", (e) => { this.logger.log("iFrame Event - click"); @@ -155,7 +164,7 @@ export class iFrame { pageNumber: ENGrid.getPageNumber(), pageCount: ENGrid.getPageCount(), giftProcess: ENGrid.getGiftProcess(), - }, "*"); + }, this.parentOrigin); } sendIframeFormStatus(status) { window.parent.postMessage({ @@ -163,7 +172,7 @@ export class iFrame { pageNumber: ENGrid.getPageNumber(), pageCount: ENGrid.getPageCount(), giftProcess: ENGrid.getGiftProcess(), - }, "*"); + }, this.parentOrigin); } getIFrameByEvent(event) { return [].slice diff --git a/packages/scripts/dist/remember-me.d.ts b/packages/scripts/dist/remember-me.d.ts index 3dd43413..0ee33880 100644 --- a/packages/scripts/dist/remember-me.d.ts +++ b/packages/scripts/dist/remember-me.d.ts @@ -36,6 +36,7 @@ export declare class RememberMe { private insertClearRememberMeLink; private getElementByFirstSelector; private insertRememberMeOptin; + private getRemoteOrigin; private useRemote; private createIframe; private clearCookie; diff --git a/packages/scripts/dist/remember-me.js b/packages/scripts/dist/remember-me.js index c1fa5549..d6a96555 100644 --- a/packages/scripts/dist/remember-me.js +++ b/packages/scripts/dist/remember-me.js @@ -47,7 +47,7 @@ export class RememberMe { if (this.useRemote()) { this.createIframe(() => { if (this.iframe && this.iframe.contentWindow) { - this.iframe.contentWindow.postMessage(JSON.stringify({ key: this.cookieName, operation: "read" }), "*"); + this.iframe.contentWindow.postMessage(JSON.stringify({ key: this.cookieName, operation: "read" }), this.getRemoteOrigin()); this._form.onSubmit.subscribe(() => { if (this.rememberMeOptIn) { this.readFields(); @@ -100,12 +100,17 @@ export class RememberMe { } updateFieldData(jsonData) { if (jsonData) { - let data = JSON.parse(jsonData); - for (let i = 0; i < this.fieldNames.length; i++) { - if (data[this.fieldNames[i]] !== undefined) { - this.fieldData[this.fieldNames[i]] = decodeURIComponent(data[this.fieldNames[i]]); + try { + let data = JSON.parse(jsonData); + for (let i = 0; i < this.fieldNames.length; i++) { + if (data[this.fieldNames[i]] !== undefined) { + this.fieldData[this.fieldNames[i]] = decodeURIComponent(data[this.fieldNames[i]]); + } } } + catch (e) { + // Malformed cookie/postMessage data; ignore and start fresh + } } } insertClearRememberMeLink() { @@ -213,6 +218,17 @@ export class RememberMe { this._events.dispatchLoad(false); window.dispatchEvent(new CustomEvent("RememberMe_Loaded", { detail: { withData: false } })); } + getRemoteOrigin() { + if (this.remoteUrl) { + try { + return new URL(this.remoteUrl).origin; + } + catch (e) { + // Fall back to wildcard if URL parsing fails + } + } + return "*"; + } useRemote() { return (!!this.remoteUrl && typeof window.postMessage === "function" && @@ -253,7 +269,7 @@ export class RememberMe { value: this.fieldData, operation: "write", expires: this.cookieExpirationDays, - }), "*"); + }), this.getRemoteOrigin()); } } readCookie() { diff --git a/packages/scripts/dist/show-hide-radio-checkboxes.js b/packages/scripts/dist/show-hide-radio-checkboxes.js index 35a8f6e9..408c74f9 100644 --- a/packages/scripts/dist/show-hide-radio-checkboxes.js +++ b/packages/scripts/dist/show-hide-radio-checkboxes.js @@ -118,11 +118,9 @@ export class ShowHideRadioCheckboxes { return; if (element.type == "radio" && element.checked) { //remove other items that have the same "class" property - state.forEach((item, index) => { - if (item.class == this.classes) { - state.splice(index, 1); - } - }); + const filteredRadioState = state.filter((item) => item.class !== this.classes); + state.length = 0; + filteredRadioState.forEach((item) => state.push(item)); //add the current item, with the currently active value state.push({ page: ENGrid.getPageID(), @@ -133,11 +131,9 @@ export class ShowHideRadioCheckboxes { } if (element.type == "checkbox") { //remove other items that have the same "class" property - state.forEach((item, index) => { - if (item.class == this.classes) { - state.splice(index, 1); - } - }); + const filteredCheckboxState = state.filter((item) => item.class !== this.classes); + state.length = 0; + filteredCheckboxState.forEach((item) => state.push(item)); //add the current item, with the first checked value or "N" if none are checked state.push({ page: ENGrid.getPageID(), diff --git a/packages/scripts/dist/upsell-lightbox.d.ts b/packages/scripts/dist/upsell-lightbox.d.ts index 94dfd13d..be931b8a 100644 --- a/packages/scripts/dist/upsell-lightbox.d.ts +++ b/packages/scripts/dist/upsell-lightbox.d.ts @@ -18,6 +18,7 @@ export declare class UpsellLightbox { private liveAmounts; private liveFrequency; private getUpsellAmount; + private evaluateSuggestion; private shouldOpen; private freqAllowed; private open; diff --git a/packages/scripts/dist/upsell-lightbox.js b/packages/scripts/dist/upsell-lightbox.js index 867f6e96..4de4358a 100644 --- a/packages/scripts/dist/upsell-lightbox.js +++ b/packages/scripts/dist/upsell-lightbox.js @@ -179,8 +179,7 @@ export class UpsellLightbox { if (upsellAmount === 0) return 0; if (typeof upsellAmount !== "number") { - const suggestionMath = upsellAmount.replace("amount", amount.toFixed(2)); - upsellAmount = parseFloat(Function('"use strict";return (' + suggestionMath + ")")()); + upsellAmount = this.evaluateSuggestion(upsellAmount, amount); } break; } @@ -189,6 +188,49 @@ export class UpsellLightbox { ? upsellAmount : this.options.minAmount; } + // Safe arithmetic evaluator for suggestion expressions like "amount / 12" or "amount * 0.1" + evaluateSuggestion(expression, amount) { + const sanitized = expression.replace(/amount/g, amount.toFixed(2)); + // Only allow digits, decimal points, whitespace, and basic math operators + if (!/^[\d\s.+\-*/()]+$/.test(sanitized)) { + this.logger.error("Invalid upsell suggestion expression: " + expression); + return 0; + } + try { + // Parse simple binary expressions: "number operator number" + const result = sanitized + .split(/([+\-*/])/) + .map((s) => s.trim()) + .filter((s) => s.length > 0) + .reduce((acc, token) => { + if (["+", "-", "*", "/"].includes(token)) { + return { value: acc.value, op: token }; + } + const num = parseFloat(token); + if (isNaN(num)) + return acc; + if (acc.op === null) + return { value: num, op: null }; + switch (acc.op) { + case "+": + return { value: acc.value + num, op: null }; + case "-": + return { value: acc.value - num, op: null }; + case "*": + return { value: acc.value * num, op: null }; + case "/": + return { value: num !== 0 ? acc.value / num : 0, op: null }; + default: + return { value: num, op: null }; + } + }, { value: 0, op: null }); + return isFinite(result.value) ? result.value : 0; + } + catch (e) { + this.logger.error("Failed to evaluate upsell suggestion: " + expression); + return 0; + } + } shouldOpen() { const upsellAmount = this.getUpsellAmount(); const paymenttype = ENGrid.getFieldValue("transaction.paymenttype") || ""; diff --git a/packages/scripts/package-lock.json b/packages/scripts/package-lock.json index 3f883d4c..10bd40d8 100644 --- a/packages/scripts/package-lock.json +++ b/packages/scripts/package-lock.json @@ -6,7 +6,7 @@ "packages": { "": { "name": "@4site/engrid-scripts", - "version": "0.23.9", + "version": "0.23.11", "license": "Unlicense", "dependencies": { "shuffle-seed": "^1.1.6", diff --git a/packages/scripts/src/app.ts b/packages/scripts/src/app.ts index 705e2480..b744de07 100644 --- a/packages/scripts/src/app.ts +++ b/packages/scripts/src/app.ts @@ -103,6 +103,7 @@ export class App extends ENGrid { private _dataLayer: DataLayer; private options: Options; + private _runRetries: number = 0; private logger = new EngridLogger("App", "black", "white", "🍏"); @@ -154,6 +155,13 @@ export class App extends ENGrid { "enjs" ) ) { + this._runRetries++; + if (this._runRetries > 50) { + this.logger.danger( + "Engaging Networks JS Framework NOT FOUND after 50 retries. Giving up." + ); + return; + } this.logger.danger("Engaging Networks JS Framework NOT FOUND"); setTimeout(() => { this.run(); @@ -498,7 +506,7 @@ export class App extends ENGrid { window.onload = (e: Event) => { this.onLoad(); if (onLoad) { - onLoad.bind(window, e); + onLoad.call(window, e); } }; } diff --git a/packages/scripts/src/events/donation-amount.ts b/packages/scripts/src/events/donation-amount.ts index 4015b52e..69ee1c5d 100644 --- a/packages/scripts/src/events/donation-amount.ts +++ b/packages/scripts/src/events/donation-amount.ts @@ -83,8 +83,10 @@ export class DonationAmount { const otherField = document.querySelector( 'input[name="' + this._other + '"]' ) as HTMLInputElement; - currentAmountValue = ENGrid.cleanAmount(otherField.value); - this.amount = currentAmountValue; + if (otherField) { + currentAmountValue = ENGrid.cleanAmount(otherField.value); + this.amount = currentAmountValue; + } } } else if ( ENGrid.checkNested( @@ -118,58 +120,64 @@ export class DonationAmount { } // Set dispatch to be checked by the SET method this._dispatch = dispatch; - // Search for the current amount on radio boxes - let found = Array.from( - document.querySelectorAll('input[name="' + this._radios + '"]') - ).filter( - (el) => el instanceof HTMLInputElement && parseInt(el.value) == amount - ); - // We found the amount on the radio boxes, so check it - if (found.length) { - const amountField = found[0] as HTMLInputElement; - amountField.checked = true; - // Change Event - const event = new Event("change", { - bubbles: true, - cancelable: true, - }); - amountField.dispatchEvent(event); - // Clear OTHER text field - this.clearOther(); - } else { - const otherField = document.querySelector( - 'input[name="' + this._other + '"]' - ) as HTMLInputElement; - if (otherField) { - const enFieldOtherAmountRadio = document.querySelector( - `.en__field--donationAmt.en__field--withOther .en__field__item:nth-last-child(2) input[name="${this._radios}"]` - ) as HTMLInputElement; - if (enFieldOtherAmountRadio) { - enFieldOtherAmountRadio.checked = true; - } - otherField.value = parseFloat(amount.toString()).toFixed(2); + try { + // Search for the current amount on radio boxes + let found = Array.from( + document.querySelectorAll('input[name="' + this._radios + '"]') + ).filter( + (el) => el instanceof HTMLInputElement && parseInt(el.value) == amount + ); + // We found the amount on the radio boxes, so check it + if (found.length) { + const amountField = found[0] as HTMLInputElement; + amountField.checked = true; // Change Event const event = new Event("change", { bubbles: true, cancelable: true, }); - otherField.dispatchEvent(event); - const otherWrapper = otherField.parentNode as HTMLElement; - otherWrapper.classList.remove("en__field__item--hidden"); + amountField.dispatchEvent(event); + // Clear OTHER text field + this.clearOther(); + } else { + const otherField = document.querySelector( + 'input[name="' + this._other + '"]' + ) as HTMLInputElement; + if (otherField) { + const enFieldOtherAmountRadio = document.querySelector( + `.en__field--donationAmt.en__field--withOther .en__field__item:nth-last-child(2) input[name="${this._radios}"]` + ) as HTMLInputElement; + if (enFieldOtherAmountRadio) { + enFieldOtherAmountRadio.checked = true; + } + otherField.value = parseFloat(amount.toString()).toFixed(2); + // Change Event + const event = new Event("change", { + bubbles: true, + cancelable: true, + }); + otherField.dispatchEvent(event); + const otherWrapper = otherField.parentNode as HTMLElement; + otherWrapper.classList.remove("en__field__item--hidden"); + } } + // Set the new amount and trigger all live variables + this.amount = amount; + } finally { + // Revert dispatch to default value (true) + this._dispatch = true; } - // Set the new amount and trigger all live variables - this.amount = amount; - // Revert dispatch to default value (true) - this._dispatch = true; } // Clear Other Field public clearOther() { const otherField = document.querySelector( 'input[name="' + this._other + '"]' ) as HTMLInputElement; + if (!otherField) return; otherField.value = ""; const otherWrapper = otherField.parentNode as HTMLElement; - otherWrapper.classList.add("en__field__item--hidden"); + if (otherWrapper) { + otherWrapper.classList.add("en__field__item--hidden"); + } } } diff --git a/packages/scripts/src/events/donation-frequency.ts b/packages/scripts/src/events/donation-frequency.ts index fea7e534..28456ff9 100644 --- a/packages/scripts/src/events/donation-frequency.ts +++ b/packages/scripts/src/events/donation-frequency.ts @@ -31,11 +31,13 @@ export class DonationFrequency { }); //Thank you page handling for utility classes if (ENGrid.getGiftProcess()) { - ENGrid.setBodyData( - "transaction-recurring-frequency", - sessionStorage.getItem("engrid-transaction-recurring-frequency") || - "onetime" - ); + let storedFrequency = "onetime"; + try { + storedFrequency = + sessionStorage.getItem("engrid-transaction-recurring-frequency") || + "onetime"; + } catch (e) {} + ENGrid.setBodyData("transaction-recurring-frequency", storedFrequency); ENGrid.setBodyData( "transaction-recurring", window.pageJson.recurring ? "y" : "n" @@ -60,10 +62,12 @@ export class DonationFrequency { this._frequency = value.toLowerCase() || "onetime"; if (this._dispatch) this._onFrequencyChange.dispatch(this._frequency); ENGrid.setBodyData("transaction-recurring-frequency", this._frequency); - sessionStorage.setItem( - "engrid-transaction-recurring-frequency", - this._frequency - ); + try { + sessionStorage.setItem( + "engrid-transaction-recurring-frequency", + this._frequency + ); + } catch (e) {} } get recurring(): string { @@ -81,9 +85,14 @@ export class DonationFrequency { // Set amount var with currently selected amount public load() { + let storedFrequency = ""; + try { + storedFrequency = + sessionStorage.getItem("engrid-transaction-recurring-frequency") || ""; + } catch (e) {} this.frequency = ENGrid.getFieldValue("transaction.recurrfreq") || - sessionStorage.getItem("engrid-transaction-recurring-frequency") || + storedFrequency || "onetime"; const recurrField = ENGrid.getField("transaction.recurrpay"); if (recurrField) { @@ -112,9 +121,12 @@ export class DonationFrequency { } // Set dispatch to be checked by the SET method this._dispatch = dispatch; - ENGrid.setFieldValue("transaction.recurrpay", recurr.toUpperCase()); - // Revert dispatch to default value (true) - this._dispatch = true; + try { + ENGrid.setFieldValue("transaction.recurrpay", recurr.toUpperCase()); + } finally { + // Revert dispatch to default value (true) + this._dispatch = true; + } } // Force a new frequency public setFrequency(freq: string, dispatch: boolean = true) { @@ -124,24 +136,27 @@ export class DonationFrequency { } // Set dispatch to be checked by the SET method this._dispatch = dispatch; - // Search for the current amount on radio boxes - let found = Array.from( - document.querySelectorAll('input[name="transaction.recurrfreq"]') - ).filter( - (el) => el instanceof HTMLInputElement && el.value == freq.toUpperCase() - ); - // We found the amount on the radio boxes, so check it - if (found.length) { - const freqField = found[0] as HTMLInputElement; - freqField.checked = true; - this.frequency = freq.toLowerCase(); - if (this.frequency === "onetime") { - this.setRecurrency("N", dispatch); - } else { - this.setRecurrency("Y", dispatch); + try { + // Search for the current amount on radio boxes + let found = Array.from( + document.querySelectorAll('input[name="transaction.recurrfreq"]') + ).filter( + (el) => el instanceof HTMLInputElement && el.value == freq.toUpperCase() + ); + // We found the amount on the radio boxes, so check it + if (found.length) { + const freqField = found[0] as HTMLInputElement; + freqField.checked = true; + this.frequency = freq.toLowerCase(); + if (this.frequency === "onetime") { + this.setRecurrency("N", dispatch); + } else { + this.setRecurrency("Y", dispatch); + } } + } finally { + // Revert dispatch to default value (true) + this._dispatch = true; } - // Revert dispatch to default value (true) - this._dispatch = true; } } diff --git a/packages/scripts/src/exit-intent-lightbox.ts b/packages/scripts/src/exit-intent-lightbox.ts index 311c545e..8fd744b2 100644 --- a/packages/scripts/src/exit-intent-lightbox.ts +++ b/packages/scripts/src/exit-intent-lightbox.ts @@ -83,17 +83,10 @@ export class ExitIntentLightbox { // user switching active program const from = e.relatedTarget; - if (!from) { - this.logger.log("Triggered by mouse position"); - this.open(); - } - - if (!this.triggerTimeout) { + if (!from && !this.triggerTimeout) { this.triggerTimeout = window.setTimeout(() => { - if (!from) { - this.logger.log("Triggered by mouse position"); - this.open(); - } + this.logger.log("Triggered by mouse position"); + this.open(); this.triggerTimeout = null; }, this.triggerDelay); } diff --git a/packages/scripts/src/iframe.ts b/packages/scripts/src/iframe.ts index bb403d3b..4e827f51 100644 --- a/packages/scripts/src/iframe.ts +++ b/packages/scripts/src/iframe.ts @@ -9,8 +9,18 @@ export class iFrame { "gray", "📡" ); + private parentOrigin: string; + constructor() { + // Derive parent origin from referrer for postMessage security + try { + this.parentOrigin = document.referrer + ? new URL(document.referrer).origin + : "*"; + } catch (e) { + this.parentOrigin = "*"; + } if (this.inIframe()) { // Add the data-engrid-embedded attribute when inside an iFrame if it wasn't already added by a script in the Page Template ENGrid.setBodyData("embedded", ""); @@ -90,7 +100,7 @@ export class iFrame { this.logger.log( `iFrame Event 'scrollTo' - Position of top of first error ${scrollTo} px` ); // check the message is being sent correctly - window.parent.postMessage({ scrollTo }, "*"); + window.parent.postMessage({ scrollTo }, this.parentOrigin); // Send the height of the iFrame window.setTimeout(() => { this.sendIframeHeight(); @@ -163,7 +173,7 @@ export class iFrame { { scroll: this.shouldScroll(), }, - "*" + this.parentOrigin ); // On click fire the resize event @@ -189,7 +199,7 @@ export class iFrame { pageCount: ENGrid.getPageCount(), giftProcess: ENGrid.getGiftProcess(), }, - "*" + this.parentOrigin ); } private sendIframeFormStatus(status: string) { @@ -200,7 +210,7 @@ export class iFrame { pageCount: ENGrid.getPageCount(), giftProcess: ENGrid.getGiftProcess(), }, - "*" + this.parentOrigin ); } private getIFrameByEvent(event: MessageEvent) { diff --git a/packages/scripts/src/remember-me.ts b/packages/scripts/src/remember-me.ts index 3a20d139..b9f8acf6 100644 --- a/packages/scripts/src/remember-me.ts +++ b/packages/scripts/src/remember-me.ts @@ -93,7 +93,7 @@ export class RememberMe { if (this.iframe && this.iframe.contentWindow) { this.iframe.contentWindow.postMessage( JSON.stringify({ key: this.cookieName, operation: "read" }), - "*" + this.getRemoteOrigin() ); this._form.onSubmit.subscribe(() => { if (this.rememberMeOptIn) { @@ -150,13 +150,17 @@ export class RememberMe { } private updateFieldData(jsonData: string) { if (jsonData) { - let data = JSON.parse(jsonData); - for (let i = 0; i < this.fieldNames.length; i++) { - if (data[this.fieldNames[i]] !== undefined) { - this.fieldData[this.fieldNames[i]] = decodeURIComponent( - data[this.fieldNames[i]] - ); + try { + let data = JSON.parse(jsonData); + for (let i = 0; i < this.fieldNames.length; i++) { + if (data[this.fieldNames[i]] !== undefined) { + this.fieldData[this.fieldNames[i]] = decodeURIComponent( + data[this.fieldNames[i]] + ); + } } + } catch (e) { + // Malformed cookie/postMessage data; ignore and start fresh } } } @@ -288,6 +292,16 @@ export class RememberMe { new CustomEvent("RememberMe_Loaded", { detail: { withData: false } }) ); } + private getRemoteOrigin(): string { + if (this.remoteUrl) { + try { + return new URL(this.remoteUrl).origin; + } catch (e) { + // Fall back to wildcard if URL parsing fails + } + } + return "*"; + } private useRemote() { return ( !!this.remoteUrl && @@ -340,7 +354,7 @@ export class RememberMe { operation: "write", expires: this.cookieExpirationDays, }), - "*" + this.getRemoteOrigin() ); } } diff --git a/packages/scripts/src/show-hide-radio-checkboxes.ts b/packages/scripts/src/show-hide-radio-checkboxes.ts index 3049ac55..f9a35a80 100644 --- a/packages/scripts/src/show-hide-radio-checkboxes.ts +++ b/packages/scripts/src/show-hide-radio-checkboxes.ts @@ -138,12 +138,12 @@ export class ShowHideRadioCheckboxes { if (element.type == "radio" && element.checked) { //remove other items that have the same "class" property - state.forEach( - (item: { page: number; class: string }, index: number) => { - if (item.class == this.classes) { - state.splice(index, 1); - } - } + const filteredRadioState = state.filter( + (item: { page: number; class: string }) => item.class !== this.classes + ); + state.length = 0; + filteredRadioState.forEach((item: { page: number; class: string }) => + state.push(item) ); //add the current item, with the currently active value @@ -158,12 +158,12 @@ export class ShowHideRadioCheckboxes { if (element.type == "checkbox") { //remove other items that have the same "class" property - state.forEach( - (item: { page: number; class: string }, index: number) => { - if (item.class == this.classes) { - state.splice(index, 1); - } - } + const filteredCheckboxState = state.filter( + (item: { page: number; class: string }) => item.class !== this.classes + ); + state.length = 0; + filteredCheckboxState.forEach( + (item: { page: number; class: string }) => state.push(item) ); //add the current item, with the first checked value or "N" if none are checked diff --git a/packages/scripts/src/upsell-lightbox.ts b/packages/scripts/src/upsell-lightbox.ts index 397b9d8b..1eeda338 100644 --- a/packages/scripts/src/upsell-lightbox.ts +++ b/packages/scripts/src/upsell-lightbox.ts @@ -240,13 +240,7 @@ export class UpsellLightbox { upsellAmount = val.suggestion; if (upsellAmount === 0) return 0; if (typeof upsellAmount !== "number") { - const suggestionMath = upsellAmount.replace( - "amount", - amount.toFixed(2) - ); - upsellAmount = parseFloat( - Function('"use strict";return (' + suggestionMath + ")")() - ); + upsellAmount = this.evaluateSuggestion(upsellAmount, amount); } break; } @@ -255,6 +249,49 @@ export class UpsellLightbox { ? upsellAmount : this.options.minAmount; } + // Safe arithmetic evaluator for suggestion expressions like "amount / 12" or "amount * 0.1" + private evaluateSuggestion(expression: string, amount: number): number { + const sanitized = expression.replace(/amount/g, amount.toFixed(2)); + // Only allow digits, decimal points, whitespace, and basic math operators + if (!/^[\d\s.+\-*/()]+$/.test(sanitized)) { + this.logger.error("Invalid upsell suggestion expression: " + expression); + return 0; + } + try { + // Parse simple binary expressions: "number operator number" + const result = sanitized + .split(/([+\-*/])/) + .map((s) => s.trim()) + .filter((s) => s.length > 0) + .reduce( + (acc: { value: number; op: string | null }, token: string) => { + if (["+", "-", "*", "/"].includes(token)) { + return { value: acc.value, op: token }; + } + const num = parseFloat(token); + if (isNaN(num)) return acc; + if (acc.op === null) return { value: num, op: null }; + switch (acc.op) { + case "+": + return { value: acc.value + num, op: null }; + case "-": + return { value: acc.value - num, op: null }; + case "*": + return { value: acc.value * num, op: null }; + case "/": + return { value: num !== 0 ? acc.value / num : 0, op: null }; + default: + return { value: num, op: null }; + } + }, + { value: 0, op: null } + ); + return isFinite(result.value) ? result.value : 0; + } catch (e) { + this.logger.error("Failed to evaluate upsell suggestion: " + expression); + return 0; + } + } private shouldOpen() { const upsellAmount = this.getUpsellAmount(); const paymenttype = ENGrid.getFieldValue("transaction.paymenttype") || "";