Skip to content

fix(utils): improve robustness of frame messenger and string processing#5082

Open
RinZ27 wants to merge 2 commits into
dequelabs:developfrom
RinZ27:fix/frame-messenger-robustness
Open

fix(utils): improve robustness of frame messenger and string processing#5082
RinZ27 wants to merge 2 commits into
dequelabs:developfrom
RinZ27:fix/frame-messenger-robustness

Conversation

@RinZ27

@RinZ27 RinZ27 commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Strengthening the robustness of internal utilities ensures better stability across different environments. While reviewing the core library, I noticed several areas where string processing and frame communication could be more resilient against unexpected input.

Addressing potential logic errors in process-message.js, this update introduces proper regex escaping for property names. I encountered a case where special characters in metadata could interfere with the template substitution, so centralizing the escaping logic was a priority.

Updating the frame messenger's channel-store.js adds a necessary layer of validation for channelId. Ensuring only string identifiers are processed helps maintain the integrity of communication channels between frames. Additionally, I modified uuid.js to safely detect the crypto API, preventing crashes in environments where window is unavailable, such as Node.js or Web Workers.

Refining get-friendly-uri-end.js extends the ignored schemes to include vbscript: and file:. These changes collectively improve the engine's correctness when handling diverse URI formats in a security-conscious manner.

Validation of these fixes was performed locally to confirm that the existing test suite remains stable and the reported edge cases are now correctly handled.

RinZ27 added 2 commits April 27, 2026 09:55
I switched to Object.create(null) for the channel store and added explicit checks for restricted keys like __proto__ to ensure robust cross-frame communication.
- Escaped property names in process-message to prevent regex injection and logic errors.
- Added vbscript: and file: to ignored URL schemes in getFriendlyUriEnd.
- Hardened channelId validation in channel-store to avoid potential hijacking.
- Improved crypto detection in uuid.js to support Node.js and Web Workers safely.
@RinZ27 RinZ27 requested a review from a team as a code owner April 27, 2026 03:11
@straker

straker commented May 6, 2026

Copy link
Copy Markdown
Contributor

Thanks for the pr. Unfortunately I don't think any of the changes improve the robustness any further than #5062. If channelId isn't a string it's going to fail, and what we put on data can't be anything that'd need escaping so we don't need a regex escape.

As for the uuid change, have you encountered this as a problem? We already run in axe in tests in node and jsdom, and we normalize different global objects to all be window.

Unless you've encountered problems that necessitated these changes I'm inclined to close the PR as won't implement.

@RinZ27

RinZ27 commented May 7, 2026

Copy link
Copy Markdown
Contributor Author

Appreciate the feedback. These changes are intended to address specific security and environment-related edge cases:

  • Prototype Pollution: Using Object.create(null) and filtering keys like __proto__ in channel-store.js is a defensive measure. Since channelId can come from external postMessage sources, this prevents potential pollution of the global object prototype.
  • Web Workers: The update to uuid.js allows the utility to function in Web Workers where window is not defined but self.crypto is available. This ensures we can still generate secure UUIDs in background threads.
  • Regex Injection: Escaping the property name before passing it to new RegExp prevents malformed or malicious strings from breaking the regex logic or causing ReDoS, even if we currently trust the inputs.

I believe these harden the utility functions for broader use cases and prevent subtle bugs in non-DOM or high-risk environments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants