Infinite-Chess / infinitechess.org

Infinite Chess Web Server
https://www.infinitechess.org
GNU Affero General Public License v3.0
186 stars 41 forks source link

Deprecate JSON APIs #150

Open BlobTheKat opened 3 months ago

BlobTheKat commented 3 months ago

JSON is very useful for small projects shared between friends but causes serious service availability problems for large scale services that recieve a wide variety of attacks. Specifically, it is possible to completely 100% CPU usage with about 3MB/s of bandwidth, a number well low enough to bypass both cloudflare and implemented rate-limits.

I propose switching to a buffer-based approach, but that requires the more active developers to familiarize themselves with a new way of handling packets.

There exists multiple APIs and libraries that facilitate this approach but by far the best to date is NPM package nanobuf (Disclaimer: I wrote it, I'm kind of a performance nerd)

Here's an example of what a websocket route rewritten from JSON to nanobuf could look like:

// Websocket
function onmessage(ws, packet){
  try{
    // ...
    const message = JSON.parse(packet)
    // ..
  }catch(e){
    // Invalid packet
    console.warn('Invalid JSON: %o', packet);
  }
  // Two vulnerabilities: What if message == null? What if message.route is not a string?
  switch(message.route){
    case 'route1':
    const value = message.value
    if(typeof value != 'number') // do something;
    ...
    break
    case 'route2':
    const thing = message.data
    handleRoute2(thing)
    ...
    default: break
  }
}
// Nanobuf

const ROUTES = Enum('route1', 'route2')

function onmessage(ws, bytes){
  if(typeof bytes == 'string') return; // Only process binary packets

  const msg = new BufReader(bytes)
  // No vulnerability: route will always be a string (either 'route1' or 'route2')
  const route = msg.enum(ROUTES)
  switch(route){
    case 'route1':
    const value = msg.i32() // Int32, a whole number, always 4 bytes
    // No type checks necessary
    ...
    break;
    case 'route2':
    // handleRoute2 can continue reading values from msg
    handleRoute2(msg)
    ...
  }
}

Optionally, nanobuf can be used in a very similar way to JSON at very little performance penalty

// Nanobuf with objects

const Schema = Struct({
  route: Enum('route1', 'route2'), // 1 byte
  username: str, // length+1 bytes
  ...
})
function onmessage(ws, bytes){
  if(typeof bytes == 'string') return // Only process binary packets

  const msg = new BufReader(bytes)
  // No vulnerability: data will always be an object with no missing properties
  const data = msg.decode(Schema)
  switch(data.route){
    case 'route1':
    doSomethingWith(data.username) // Again, no type checks necessary
    ...
    break;
    case 'route2':
    // handleRoute2 can continue reading values from msg
    handleRoute2(data, msg)
    ...
  }
}

const Route2Schema = Struct({
  privateGame: bool, rated: bool,
  timeControl1: f64, /* float64, i.e double, i.e number */
  timeControl2: f32, /* float32, less precision but smaller size */
  opponent: Optional(str) // string or null
})
function handleRoute2(data, msg){
  const matchDetails = msg.decode(Route2Schema)
  // matchDetails => {privateGame: ..., rated: ..., timeControl1: ..., ...}
  // TODO: start match
}

This method can parse varying payloads anywhere between 2-50 times faster than JSON Additionally it is a lot harder to omit type checks by accident (since the underlying format does not support dynamic typing), reducing the risk of exploits

Naviary2 commented 3 months ago

This would be a smart upgrade soon! The server is JSON-parsing every single websocket message right now, no matter its size. I could see how that could easily be exploited by sending data that takes very long to parse.

I have another note, websocket messages are not the only place where json data from the user is parsed. There are all sorts of form submissions, like login, create account, etc, that also submit form data in their http post request.

BlobTheKat commented 3 months ago

HTTP APIs are a little tricker to exploit as cloudflare is able to (occasionally) detect them but yeah that also seems like a good thing to do. Node makes it very easy to respond to requests with binary data, and web APIs make it easy to parse the response too.

Naviary2 commented 3 months ago

Does Nanobuf allow strings of arbitrary or very large size? In the future, maximum move distance will be greatly increased, making use of BigInts, and those will need to be sent over websocket as well.

BlobTheKat commented 3 months ago

Does Nanobuf allow strings of arbitrary or very large size? In the future, maximum move distance will be greatly increased, making use of BigInts, and those will need to be sent over websocket as well.

Nanobuf supports strings, arrays and will soon support BigInt