From 1026f18c085e72ecbac8db860dfe6476948f5bca Mon Sep 17 00:00:00 2001 From: William Mantly Date: Wed, 15 Jul 2026 00:45:24 -0400 Subject: [PATCH] Rate-limit host-mutating routes CodeQL flagged POST/PUT/DELETE /api/host* as missing rate limiting despite performing authorization -- same authLimiter pattern routes/auth.js already uses, applied here with a higher ceiling since legitimate admin work (bulk edits) is expected on these routes. CodeQL also flagged utils/basicauth.js's SHA-1 hashing as reachable from the new basicauth-user route -- this is the existing, documented htpasswd- compatible {SHA} scheme (see the comment on hashPassword), not something this PR changes; left as-is per that comment's existing "follow-up" note, since swapping it requires a coordinated change to ops/nginx_conf/hostfeatures.lua's verification and a migration path for already-stored hashes. Co-Authored-By: Claude Sonnet 5 --- nodejs/routes/host.js | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/nodejs/routes/host.js b/nodejs/routes/host.js index c93e1f4..775d3d4 100755 --- a/nodejs/routes/host.js +++ b/nodejs/routes/host.js @@ -1,6 +1,7 @@ 'use strict'; const router = require('express').Router(); +const {rateLimit} = require('express-rate-limit'); const conf = require('@simpleworkjs/conf'); const {Host, Domain, User} = require('../models').models; const {LocalGroup} = require('../models/local_group'); @@ -12,6 +13,20 @@ const {hashBasicAuthUsers} = require('../utils/basicauth'); const Model = Host; +// Throttle host-mutating endpoints (create/update/delete a host, manage a +// basic-auth user's password) per IP. These already require an authenticated, +// authorized manager/admin, but a compromised or careless session shouldn't +// be able to hammer them unboundedly — same pattern as routes/auth.js's +// authLimiter, just a higher ceiling since legitimate admin work (bulk edits) +// is expected here. +const mutateLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, // 15 minutes + max: 300, // 300 mutations per IP per window + standardHeaders: true, + legacyHeaders: false, + message: {name: 'TooManyRequests', message: 'Too many requests, please try again later.'}, +}); + // Reject a malformed host/target before it reaches the model. Throws a 422 // ObjectValidateError (per-field keys) that the frontend surfaces inline. function validateHostFields(body){ @@ -83,7 +98,7 @@ router.get('/', async function(req, res, next){ } }); -router.post('/', authz.requireDomainRole('manager', authz.resolve.hostBody), async function(req, res, next){ +router.post('/', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostBody), async function(req, res, next){ try{ req.body.created_by = authz.reqUsername(req); validateHostFields(req.body); @@ -125,7 +140,7 @@ router.get('/lookupobj', authz.requireAdmin, async function(req, res, next){ } }); -router.delete('/cache', authz.requireAdmin, async function(req, res, next){ +router.delete('/cache', mutateLimiter, authz.requireAdmin, async function(req, res, next){ try{ let count = await Model.clearCache(); @@ -150,7 +165,7 @@ router.get('/:item', authz.requireDomainRole('viewer', authz.resolve.hostParam), } }); -router.put('/:item', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ +router.put('/:item', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ try{ req.body.updated_by = authz.reqUsername(req); validateHostFields(req.body); @@ -172,7 +187,7 @@ router.put('/:item', authz.requireDomainRole('manager', authz.resolve.hostParam) } }); -router.delete('/:item', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ +router.delete('/:item', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ try{ let item = await Model.get(req.params.item); let count = await item.remove(); @@ -192,7 +207,7 @@ router.delete('/:item', authz.requireDomainRole('manager', authz.resolve.hostPar // textarea there means "leave existing users untouched", see // normalizeHostFeatures), which makes deleting or rotating one user's // password error-prone from that form. These two routes touch exactly one key. -router.put('/:item/basicauth-user/:username', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ +router.put('/:item/basicauth-user/:username', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ try{ let item = await Model.get(req.params.item); let sanitized = sanitizeBasicAuthObject({[req.params.username]: req.body.password}); @@ -210,7 +225,7 @@ router.put('/:item/basicauth-user/:username', authz.requireDomainRole('manager', } }); -router.delete('/:item/basicauth-user/:username', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ +router.delete('/:item/basicauth-user/:username', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ try{ let item = await Model.get(req.params.item); let users = Object.assign({}, item.basicauth_users); @@ -223,7 +238,7 @@ router.delete('/:item/basicauth-user/:username', authz.requireDomainRole('manage } }); -router.put('/:item/renew', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ +router.put('/:item/renew', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ try{ let item = await Model.get(req.params.item); item.createWildcardCert();