Signal K Server Vulnerable to Denial of Service via Unrestricted Access Request Flooding

@@ -97,6 +97,7 @@
     "express": "^4.10.4",
     "express-easy-zip": "^1.1.5",
     "express-namespace": "^0.1.1",
+    "express-rate-limit": "^8.2.1",
     "figlet": "^1.2.0",
     "file-timestamp-stream": "^2.1.2",
     "geolib": "3.2.2",

@@ -54,13 +54,30 @@ import { WithWrappedEmitter } from './events'
 import { getAISShipTypeName } from '@signalk/signalk-schema'
 import availableInterfaces from './interfaces'
 import redirects from './redirects.json'
+import rateLimit from 'express-rate-limit'
 
 const readdir = util.promisify(fs.readdir)
 const debug = createDebug('signalk-server:serverroutes')
 const ncp = ncpI.ncp
 const defaultSecurityStrategy = './tokensecurity'
 const skPrefix = '/signalk/v1'
 
+const apiLimiter = rateLimit({
+  windowMs: 10 * 60 * 1000, // 10 minutes
+  max: 100,
+  message: {
+    message: 'Too many requests from this IP, please try again after 10 minutes'
+  }
+})
+
+const loginStatusLimiter = rateLimit({
+  windowMs: 10 * 60 * 1000, // 10 minutes
+  max: 10,
+  message: {
+    message: 'Too many requests from this IP, please try again after 10 minutes'
+  }
+})
+
 interface ScriptsApp {
   addons: ModuleInfo[]
   pluginconfigurators: ModuleInfo[]
@@ -224,9 +241,9 @@ module.exports = function (
     res.json(result)
   }
 
-  app.get(`${SERVERROUTESPREFIX}/loginStatus`, getLoginStatus)
+  app.get(`${SERVERROUTESPREFIX}/loginStatus`, loginStatusLimiter, getLoginStatus)
   //TODO remove after a grace period
-  app.get(`/loginStatus`, (req: Request, res: Response) => {
+  app.get(`/loginStatus`, loginStatusLimiter, (req: Request, res: Response) => {
     console.log(
       `/loginStatus is deprecated, try updating webapps to the latest version`
     )
@@ -459,7 +476,14 @@ module.exports = function (
     }
   )
 
-  app.post(`${skPrefix}/access/requests`, (req: Request, res: Response) => {
+  app.post(`${skPrefix}/access/requests`, apiLimiter, (req: Request, res: Response) => {
+    if (
+      req.headers['content-length'] &&
+      parseInt(req.headers['content-length']) > 10 * 1024
+    ) {
+      res.status(413).send('Payload too large')
+      return
+    }
     const config = getSecurityConfig(app)
     const ip = req.headers['x-forwarded-for'] || req.connection.remoteAddress
     if (!app.securityStrategy.requestAccess) {
@@ -478,12 +502,12 @@ module.exports = function (
       })
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       .catch((err: any) => {
-        console.log(err.stack)
-        res.status(500).send(err.message)
+        console.error(err.message)
+        res.status(err.statusCode || 500).send(err.message)
       })
   })
 
-  app.get(`${skPrefix}/requests/:id`, (req: Request, res: Response) => {
+  app.get(`${skPrefix}/requests/:id`, apiLimiter, (req: Request, res: Response) => {
     queryRequest(req.params.id)
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       .then((reply: any) => {

@@ -178,6 +178,16 @@ module.exports = function (app, config) {
   }
 
   function setupApp() {
+    const rateLimit = require('express-rate-limit')
+    const loginLimiter = rateLimit({
+      windowMs: 10 * 60 * 1000, // 10 minutes
+      max: 10, // Limit each IP to 10 login requests per windowMs
+      message: {
+        message:
+          'Too many login attempts from this IP, please try again after 10 minutes'
+      }
+    })
+
     app.use(require('body-parser').urlencoded({ extended: true }))
 
     app.use(require('cookie-parser')())
@@ -194,7 +204,7 @@ module.exports = function (app, config) {
       return dest
     }
 
-    app.post(['/login', `${skAuthPrefix}/login`], (req, res) => {
+    app.post(['/login', `${skAuthPrefix}/login`], loginLimiter, (req, res) => {
       const name = req.body.username
       const password = req.body.password
       const remember = req.body.rememberMe
@@ -1083,6 +1093,12 @@ module.exports = function (app, config) {
 
   strategy.requestAccess = (theConfig, clientRequest, sourceIp, updateCb) => {
     return new Promise((resolve, reject) => {
+      if (filterRequests('accessRequest', 'PENDING').length >= 100) {
+        const err = new Error('Too many pending access requests')
+        err.statusCode = 503
+        reject(err)
+        return
+      }
       createRequest(
         app,
         'accessRequest',

@@ -0,0 +1,108 @@
+const chai = require('chai')
+chai.Should()
+const { startServerP } = require('./servertestutilities')
+const { freeport } = require('./ts-servertestutilities')
+
+describe('Rate Limiting', () => {
+  let server, url, port
+
+  before(async function () {
+    port = await freeport()
+    url = `http://0.0.0.0:${port}`
+    const securityConfig = {
+      allowNewUserRegistration: true,
+      allowDeviceAccessRequests: true
+    }
+    server = await startServerP(port, true, {}, securityConfig)
+  })
+
+  after(async function () {
+    await server.stop()
+  })
+
+  it('should limit login attempts', async function () {
+    const requests = []
+    for (let i = 0; i < 10; i++) {
+      requests.push(
+        fetch(`${url}/signalk/v1/auth/login`, {
+          method: 'POST',
+          headers: { 'Content-Type': 'application/json' },
+          body: JSON.stringify({
+            username: 'admin',
+            password: 'wrongpassword'
+          })
+        })
+      )
+    }
+
+    await Promise.all(requests)
+
+    const res = await fetch(`${url}/signalk/v1/auth/login`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({
+        username: 'admin',
+        password: 'wrongpassword'
+      })
+    })
+
+    res.status.should.equal(429)
+  })
+
+  it('should limit access requests', async function () {
+    const requests = []
+    for (let i = 0; i < 100; i++) {
+      requests.push(
+        fetch(`${url}/signalk/v1/access/requests`, {
+          method: 'POST',
+          headers: { 'Content-Type': 'application/json' },
+          body: JSON.stringify({
+            clientId: `device-${i}`,
+            description: `Device ${i}`
+          })
+        })
+      )
+    }
+
+    await Promise.all(requests)
+
+    const res = await fetch(`${url}/signalk/v1/access/requests`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({
+        clientId: 'device-101',
+        description: 'Device 101'
+      })
+    })
+
+    res.status.should.equal(429)
+  })
+
+  it('should limit request status checks', async function () {
+    const requests = []
+    for (let i = 0; i < 100; i++) {
+      requests.push(
+        fetch(`${url}/signalk/v1/requests/123`)
+      )
+    }
+
+    await Promise.all(requests)
+
+    const res = await fetch(`${url}/signalk/v1/requests/123`)
+    res.status.should.equal(429)
+  })
+
+  it('should limit login status checks', async function () {
+    const requests = []
+    for (let i = 0; i < 10; i++) {
+      requests.push(
+        fetch(`${url}/loginStatus`)
+      )
+    }
+
+    await Promise.all(requests)
+
+    const res = await fetch(`${url}/loginStatus`)
+    res.status.should.equal(429)
+  })
+})

@@ -565,4 +565,67 @@ describe('Security', () => {
     json = await result.json()
     json.length.should.equal(1)
   })
+
+  it('should reject access requests > 10kb', async function () {
+    const largeDescription = 'a'.repeat(10 * 1024)
+    const res = await fetch(`${url}/signalk/v1/access/requests`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({
+        clientId: 'device-large',
+        description: largeDescription
+      })
+    })
+    res.status.should.equal(413)
+  })
+})
+
+describe('Access Request Limit', () => {
+  let server, url, port
+
+  before(async function () {
+    this.timeout(20000)
+    port = await freeport()
+    url = `http://0.0.0.0:${port}`
+    const securityConfig = {
+      allowNewUserRegistration: true,
+      allowDeviceAccessRequests: true
+    }
+    server = await startServerP(port, true, {}, securityConfig)
+  })
+
+  after(async function () {
+    await server.stop()
+  })
+
+  it('should limit pending access requests to 100', async function () {
+    this.timeout(20000)
+    const requests = []
+    for (let i = 0; i < 100; i++) {
+      requests.push(
+        fetch(`${url}/signalk/v1/access/requests`, {
+          method: 'POST',
+          headers: { 'Content-Type': 'application/json' },
+          body: JSON.stringify({
+            clientId: `device-${i}`,
+            description: `Device ${i}`
+          })
+        })
+      )
+    }
+
+    await Promise.all(requests)
+
+    const res = await fetch(`${url}/signalk/v1/access/requests`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({
+        clientId: 'device-101',
+        description: 'Device 101'
+      })
+    })
+
+    res.status.should.equal(503)
+  })
 })
+