Simple websocket chat app, security and efficiency concerns

Question

I'm coding a simple websocket chat app. I was wondering if my code is secure and efficient. I also have a question about the way how I'm sending objects and then checking the type, is there anything better/efficient I can do? Such as making my own events?

Please tell me any improvements I can do.

This is the chat.js (client side script):

I'm most concerned about this line: ws.send(JSON.stringify({type:"connectBoth", senderid:res.receiverid, receiverid:res.senderid}))

There is no direct input into that line, but can't they just open console and go execute that function themselves? With their own parameters they choose, and thus result in a connection with somoene that didn't agree?

let id = document.getElementById("chatid");
function copyID(){
    navigator.clipboard.writeText(id.innerHTML);

    alert("Copied!");
}


(function() {
    const sendBtn = document.getElementById("send");
    const messages = document.getElementById("chatModule");
    const messageBox = document.getElementById("message");
    const friendIDBox = document.getElementById("friend-id");
    const connectBtn = document.getElementById("connect");
    const connStatus = document.getElementById("conn-status");

    let ws;

    

    function showMessage(message, side){
        var newMessage = document.createElement("div");
        if(side == "sender"){
            newMessage.setAttribute("class", "sendermessage");
        }
        else if(side == "receiver"){
            newMessage.setAttribute("class", "receivermessage"); 
        }
        newMessage.textContent = message;
        messages.appendChild(newMessage);
        messageBox.value = '';
    }

    function init(){
        if(ws){
            ws.onerror = ws.onopen = ws.onclose = null;
            ws.close();
        }

        ws = new WebSocket('ws://localhost:8080/?token=2121');
        ws.onopen = () => {
            console.log('Connection opened!')
        }
        // ws.onmessage =  ({ data }) => showMessage(data, "receiver");
        ws.onmessage = object => {
            res = JSON.parse(object.data);
            console.log(res);
            switch(res.type){
                case "id":
                    id.innerText = res.data;
                break;
                case "connectionPrompt":
                    let agree = confirm("Do you want to connect with: " + res.senderid);
                    if(agree){
                        ws.send(JSON.stringify({type:"connectBoth", senderid:res.receiverid, receiverid:res.senderid}))
                        connStatus.innerText = "Connection status: Connected to " + res.senderid;
                    }
                break;
                case "connAccepted":
                    connStatus.innerText = "Connection status: Connected to " + res.senderid;
                break;
                case "msg":
                    showMessage(res.msg, "receiver");
                break;

            }
        } 
        ws.onclose = function(){
            ws = null;
        }
    }

    sendBtn.onclick = () => {
        if(!ws){
            showMessage("No WebSocket connection!");
            return;
        }

        ws.send(JSON.stringify({type:"msg", data:messageBox.value}));
        showMessage(messageBox.value, "sender");
    } 

    connectBtn.onclick = () => {
        ws.send(JSON.stringify({type:"connectRequest", data:friendIDBox.value}));
        connStatus.innerText = "Connection status: Pending..."
        alert("Request sent!");
    }

    

    init();
})();

This is the server.js (server side-script)

const express = require('express');
const { json } = require('express/lib/response');
const http = require('http');
const WebSocket = require('ws');
//Web server socket = wss
const port = 8080;
const server = http.createServer(express);
const wss = new WebSocket.Server({ server });




function heartbeat() {
    this.isAlive = true;
}

function generateID(){
    return Math.floor((1 + Math.random()) * 0x10000).toString(16).substring(1);
}

wss.getUniqueID = () => {
    return generateID() + generateID() + generateID();
}
    


wss.on("connection", function connection(ws){
    ws.id = wss.getUniqueID();
    ws.send(JSON.stringify({type:"id", data: ws.id}));
    console.log("New client connected with id: " + ws.id);
    ws.isAlive = true;
    ws.on('pong', heartbeat);
    ws.on("message", function incoming(data, isBinary){
        let message = isBinary ? data : data.toString();
        message = JSON.parse(message);
        console.log(message);
        switch(message.type){
            case "connectRequest":
                wss.clients.forEach(client => {
                    if(client.id == message.data)
                    client.send(JSON.stringify({type:"connectionPrompt", senderid:ws.id, receiverid:message.data }))
                })
                // ws.friendid = message.data;

            break;
            case "connectBoth":
                ws.friendid = message.receiverid;
                wss.clients.forEach(client => {
                    if(client.id == message.receiverid){
                        client.friendid = message.senderid;
                        client.send(JSON.stringify({type:"connAccepted", senderid:message.senderid}))
                    }
                })

            break;
            case "msg":
                wss.clients.forEach(client => {
                    if(client != ws && client.readyState == WebSocket.OPEN){
                        console.log("Client id: " + client.id + " and " + client.friendid);
                        if (client.id == ws.friendid){
                            client.send(JSON.stringify({type:"msg", msg:message.data}));
                        }
                   }
                    
                })
            break;
            
        }
        
        
        
        // const message = isBinary ? data : data.toString();
        // wss.clients.forEach(function each(client){
        //     if(client != ws && client.readyState == WebSocket.OPEN){
        //         client.send(message);
        //     }
        // })
    })
    

})

const interval = setInterval(function ping() {
    wss.clients.forEach(function each(ws) {
      if (ws.isAlive === false) return ws.terminate();
  
      ws.isAlive = false;
      ws.ping();
    });
  }, 1000);
  

  wss.on('close', function close() {
    clearInterval(interval);
    console.log("Connection lost!");
  });
  

server.listen(port, function(){
    console.log('Server is listening on ' + port);
});

Thanks, Kind Regards!

Answer 1

There is no direct input into that line, but can't they just open console and go execute that function themselves?

Yes, but if I was a malicious actor, I would just take your websocket url and build my own script based off of yours.

How can you stop this?

You can't.

But you can make it more complex to use to dissuade malicious actors.

---

Before we get into how to make things more complex, lets first ask the question.

"What are the potential damages a malicious actor can do?"

1. ✅ You're not using a database... so no sql/no-sql injections are possible
2. ⚠️ Users don't seem to be authenticated... A user could potentially create an infinite amount of websocket connections.
3. ⚠️ Users don't seem to be rate limited... A user could poetentially send an infinite amount of messages within a short amount of time.
4. ⚠️ Messages are forwarded to all clients and inserted in DOM, potential XSS (if innerHTML is used)

---

Require users to be authenticated

1. Require users to have an account
2. Generate a webtoken to be used to authenticate the user for the initial websocket handhake
3. Set rate limit per account, to ensure that they cannot brute force a hack

This will allow you better identify a user and (if necessary) to properly ban/suspend them (forcing them to create a new email address, get verified, etc).

---

XSS attacks

You seem to be using textContent which is already a good habit and will allow avoiding XSS attacks. Definitely do not use innerHTML. However, if you would like users to write formatted text.. consider, sanitizing incoming messages.