Skip to content
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.

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

Sanitization/Renaming for Security in Snapshot vs Replay reconstruction #1528

Open
sdemjanenko opened this issue Jul 1, 2024 · 1 comment

Comments

@sdemjanenko
Copy link

sdemjanenko commented Jul 1, 2024

I was reviewing the security of the Replayer, specifically the scenario where a bad actor POSTs a fake recording to a recording endpoint. Looking at the logic of the snapshot, i see a lot of code to rename/strip potentially dangerous attributes. An example is

if (isScript) {
  textContent = 'SCRIPT_PLACEHOLDER';
}

I did not observe similar logic in the Replayer to ensure that a script tag isn't inserted (or if it is, that the textContent is SCRIPT_PLACEHOLDER). This means that the fake recording could be constructed to have a script tag with a body of JS that performs some bad activity.

This leads me to a few design questions:

  • why is the code sanitizing on recording versus sanitizing the replayer right before it injects the recorded DOM into the iframe's HTML?
  • what attributes need to be sanitized to protect the html that is inserted (and perhaps there is an opportunity to have a callback fn in the Replayer to mutate a node before insertion).
  • does the CSP of the recorded site need to be stored and then inserted into the iframe? It would be unfortunate if a site went through the pain of setting up a CSP just to have a bypass happen via the Replayer (exfiltrating some sensitive data).

I haven't had the time to write PoCs for any of this yet, but I wanted to open a discussion into organization of the logic of snapshotting + replayer. More logic may need to be moved into the Replayer for security purposes. As a general rule, I think about how the replayer may be able to receive completely untrusted events and insert them "safely" into the DOM.

@eoghanmurray
Copy link
Contributor

I believe the SCRIPT_PLACEHOLDER marker isn't so much sanitization, as reducing the size of the recording (see also 'slimDOM' options that I authored; however the SCRIPT_PLACEHOLDER was there well before that).

The main protection from malicious recordings is via the sandbox attribute on the replayer iframe (which must be bypassed for canvas replay; see UNSAFE_replayCanvas).

I'm not sure what sort of exfiltration you are thinking of that the CSP policy would protect you from, but I imagine it's not possible when scripting is disabled in the iframe?

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

No branches or pull requests

2 participants