XSS through editor: _setProperty _isGlobal allows arbitrary JS execution

[unlox775-code-dot-org]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL
• Build process
• Unit testing
• Internationalization
• Friendly errors
• Other (specify if possible)

p5.js version

1.11.1 and 1.3.0

Web browser and version

131.0.6778.109 (Official Build) (arm64)

Operating system

MacOSX

Steps to reproduce this

We use this as an online educational portal where one student can create using the p5 editor. We rely on p5 to limit the functions that can be called.

The problem comes in that they can share their creation with another student. This means that the code input through p5, gets executed on a different user's browser. This allows for compromise of security tokens in cookies, local storage, etc.

Steps:

1. Student A creates and saves a project with the below exploit
2. Student A shares the project URL with another user, Student B
3. When Student B opens the project (not the editor in this case, as they are not the owner), and views the project
4. When the project is viewed by Student B, the exploit runs, and can examine/exfiltrate cookies, etc.

Snippet:

function draw() {
  _setProperty("_isGlobal", true);
  _setProperty("location", "javascript:console.log(document.cookie.length)");
  _setProperty("_isGlobal", false); 
}

[welcome]

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

[davepagurek]

Would this be any different than the student avoiding _setProperty and just logging the value directly?

function draw() {
  console.log(document.cookie.length);
}

I'm not sure this is any more of an exploit than just sending another student a page that does the same thing without using any p5 methods, in which case this would not really be a problem for p5 to fix. MDN suggests using HTTP-only cookies to avoid having them be readable by JavaScript at all.

[davepagurek]

Also, document.cookie only sees cookies associated with the page you are on. It looks like the p5 web editor doesn't have JS-readable cookies (so document.cookie.length is 0 on web editor pages) after a quick test, and cookies from other pages can't be read this way due to the browser's built in security policies.

Similarly for local storage, they would only possibly be able to read storage from the same domain. If a fullscreen editor.p5js.org link is shared, that means they will only be able to access localStorage set by other editor.p5js.org sketches. Ideally it would be scoped to just the current sketch, but as it is sharing the storage with the whole domain, I think the security impact of that is fairly minimal. Probably the lesson there is to not store sensitive data in localStorage if hosting content on a shared domain like editor.p5js.org.

[unlox775-code-dot-org]

The point of the vulnerability is not that it allows document.cookies. The point is that it allows the entire JavaScript language, including dangerous things they could get access to which cookie is one of many sensitive actions they could do. The fundamental nature of XSS is that they can impersonate and do anything the attacked user session has access to do (within the context of the same site browser guardrails).

Other actions they could easily perform:

• Delete another user's account
• Get a site admin to visit the page, and gain full admin control of the platform

We us P5.JS on our education platform which has authentication and other features to track user sessions. The issue here is that we are relying on P5 to reduce users to a subset of functions, not all of Javascript. While their code runs on other authenticated users browsers a full access to JavaScript would allow them to completely the compromise other users of our platform.

The intended use case is that our students are allowed to only access the documented functions of P5. Is there a way the _setProperty function can be disallowed? Or perhaps even that if _setProperty is called, with the arg "_isGlobal", that it errors out?

An error like this:

[davepagurek]

Without getting into specifics yet, probably the way to embed users' sketches to sidestep these concerns would be to load it in an <iframe> tag where you can use the sandbox attribute to limit its access to session info from the outside page: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe#sandbox

The reason why I suggest that is that a user's p5 sketch is already running arbitrary user-specified code. So anything a user could access via _setProperty, they could also access by just putting that code directly in draw without _setProperty. Are you currently blocking access to other properties in some way, e.g. limiting the ability to access the document variable and its cookies property? If so, could you use that same method to block _setProperty? If you aren't, then I think there is still a security hole to be fixed even if _setProperty didn't exist, as this sketch would still access the same variable by logging it in their code.

By running all the code in a sandboxed iframe, maybe just <iframe sandbox="allowscripts"> and nothing else, then users can still access document.cookies and other things in the page, but they will be empty. Essentially, the code is run in a fresh session.

[unlox775-code-dot-org]

Yes, I think I made this issue report in error. Thank you so much for helping me!

It turns out it was another library that was preventing access to document.* and other javascript functions. P5 is working fine! ❤️