fix(security): add authorization checks to prevent cross-account access (CWE-639) (#558)

- Add precondition support to decorate.js retrieve function
- Fix google-custom-voices.js typo and add delete precondition
- Check ownership via speech_credential for google-custom-voices
- Add retrieve/delete preconditions to lcr-carrier-set-entries.js
- Add retrieve precondition to sip-gateways.js and smpp-gateways.js
- Add scope check to lcr-routes.js custom GET handler
- Add full authorization to tenants.js for all CRUD operations
- Add scoped query methods to tenant model

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

lib/models/tenant.js

@@ -1,9 +1,27 @@
 const Model = require('./model');
+const {promisePool} = require('../db');
 
 class MsTeamsTenant extends Model {
   constructor() {
     super();
   }
+
+  static async retrieveAll(account_sid) {
+    if (account_sid) {
+      const sql = `SELECT * FROM ${this.table} WHERE account_sid = ?`;
+      const [rows] = await promisePool.query(sql, account_sid);
+      return rows;
+    }
+    const sql = `SELECT * FROM ${this.table}`;
+    const [rows] = await promisePool.query(sql);
+    return rows;
+  }
+
+  static async retrieveAllByServiceProviderSid(service_provider_sid) {
+    const sql = `SELECT * FROM ${this.table} WHERE service_provider_sid = ?`;
+    const [rows] = await promisePool.query(sql, service_provider_sid);
+    return rows;
+  }
 }
 
 MsTeamsTenant.table = 'ms_teams_tenants';

lib/routes/api/decorate.js

@@ -53,10 +53,14 @@ function add(router, klass, preconditions) {
   });
 }
 
-function retrieve(router, klass) {
+function retrieve(router, klass, preconditions) {
   router.get('/:sid', async(req, res) => {
     const logger = req.app.locals.logger;
     try {
+      if ('retrieve' in preconditions) {
+        assert(typeof preconditions.retrieve === 'function');
+        await preconditions.retrieve(req, req.params.sid);
+      }
       const results = await klass.retrieve(req.params.sid);
       if (results.length === 0) return res.sendStatus(404);
       return res.status(200).json(results[0]);

lib/routes/api/google-custom-voices.js

@@ -37,34 +37,64 @@ const validateUpdate = async(req) => {
   }
 };
 
+const validateRetrieveOrDelete = async(req, sid) => {
+  const googleVoice = await GoogleCustomVoice.retrieve(sid);
+  if (!googleVoice || googleVoice.length === 0) {
+    throw new DbErrorBadRequest('not found');
+  }
+  const voice = googleVoice[0];
+
+  // Check ownership via the linked speech credential
+  const credential = await SpeechCredential.retrieve(voice.speech_credential_sid);
+  if (!credential || credential.length === 0) {
+    throw new DbErrorBadRequest('invalid speech_credential_sid');
+  }
+  const cred = credential[0];
+
+  if (req.user.hasServiceProviderAuth && cred.service_provider_sid !== req.user.service_provider_sid) {
+    throw new DbErrorForbidden('Insufficient privileges');
+  }
+  if (req.user.hasAccountAuth && cred.account_sid !== req.user.account_sid) {
+    throw new DbErrorForbidden('Insufficient privileges');
+  }
+};
+
 const preconditions = {
   add: validateAdd,
   update: validateUpdate,
+  delete: validateRetrieveOrDelete,
 };
 
-decorate(router, GoogleCustomVoice, ['add', 'retrieve', 'update', 'delete'], preconditions);
+decorate(router, GoogleCustomVoice, ['add', 'update', 'delete'], preconditions);
 
 const voiceCloningKeySubString = (voice_cloning_key) => {
   return voice_cloning_key ? voice_cloning_key.substring(0, 100) + '...' : undefined;
 };
 
-router.get('/: sid', async(req, res) => {
+router.get('/:sid', async(req, res) => {
   const logger = req.app.locals.logger;
   try {
     const {sid} = req.params;
-    const account_sid = req.user.account_sid;
-    const service_provider_sid = req.user.service_provider_sid;
 
-    const google_voice = await GoogleCustomVoice.retrieve(sid);
-    google_voice.voice_cloning_key = voiceCloningKeySubString(google_voice.voice_cloning_key);
-    if (!google_voice) {
+    const results = await GoogleCustomVoice.retrieve(sid);
+    if (!results || results.length === 0) {
       return res.sendStatus(404);
     }
-    if (req.user.hasScope('service_provider') && google_voice.service_provider_sid !== service_provider_sid ||
-      req.user.hasScope('account') && google_voice.account_sid !== account_sid) {
-      throw new DbErrorForbidden('Insufficient privileges');
+    const google_voice = results[0];
+
+    // Check ownership via the linked speech credential
+    const credential = await SpeechCredential.retrieve(google_voice.speech_credential_sid);
+    if (credential && credential.length > 0) {
+      const cred = credential[0];
+      if (req.user.hasServiceProviderAuth && cred.service_provider_sid !== req.user.service_provider_sid) {
+        throw new DbErrorForbidden('Insufficient privileges');
+      }
+      if (req.user.hasAccountAuth && cred.account_sid !== req.user.account_sid) {
+        throw new DbErrorForbidden('Insufficient privileges');
+      }
     }
 
+    google_voice.voice_cloning_key = voiceCloningKeySubString(google_voice.voice_cloning_key);
     return res.status(200).json(google_voice);
   } catch (err) {
     sysError(logger, res, err);
@@ -109,16 +139,23 @@ router.get('/', async(req, res) => {
 router.post('/:sid/VoiceCloningKey', upload.single('file'), async(req, res) => {
   const {logger} = req.app.locals;
   const {sid} = req.params;
-  const account_sid = req.user.account_sid;
-  const service_provider_sid = req.user.service_provider_sid;
   try {
-    const google_voice = await GoogleCustomVoice.retrieve(sid);
-    if (!google_voice) {
+    const results = await GoogleCustomVoice.retrieve(sid);
+    if (!results || results.length === 0) {
       return res.sendStatus(404);
     }
-    if (req.user.hasScope('service_provider') && google_voice.service_provider_sid !== service_provider_sid ||
-      req.user.hasScope('account') && google_voice.account_sid !== account_sid) {
-      throw new DbErrorForbidden('Insufficient privileges');
+    const google_voice = results[0];
+
+    // Check ownership via the linked speech credential
+    const credential = await SpeechCredential.retrieve(google_voice.speech_credential_sid);
+    if (credential && credential.length > 0) {
+      const cred = credential[0];
+      if (req.user.hasServiceProviderAuth && cred.service_provider_sid !== req.user.service_provider_sid) {
+        throw new DbErrorForbidden('Insufficient privileges');
+      }
+      if (req.user.hasAccountAuth && cred.account_sid !== req.user.account_sid) {
+        throw new DbErrorForbidden('Insufficient privileges');
+      }
     }
 
     const voice_cloning_key = Buffer.from(fs.readFileSync(req.file.path)).toString();

lib/routes/api/lcr-carrier-set-entries.js

@@ -1,8 +1,9 @@
 const router = require('express').Router();
 const LcrCarrierSetEntry = require('../../models/lcr-carrier-set-entry');
 const LcrRoute = require('../../models/lcr-route');
+const Lcr = require('../../models/lcr');
 const decorate = require('./decorate');
-const {DbErrorBadRequest} = require('../../utils/errors');
+const {DbErrorBadRequest, DbErrorForbidden} = require('../../utils/errors');
 const sysError = require('../error');
 
 const validateAdd = async(req) => {
@@ -43,9 +44,48 @@ const validateUpdate = async(req) => {
   }
 };
 
+const validateRetrieveOrDelete = async(req, sid) => {
+  // Get the entry
+  const entries = await LcrCarrierSetEntry.retrieve(sid);
+  if (!entries || entries.length === 0) {
+    throw new DbErrorBadRequest('not found');
+  }
+  const entry = entries[0];
+
+  // Get the lcr_route
+  const routes = await LcrRoute.retrieve(entry.lcr_route_sid);
+  if (routes.length === 0) {
+    throw new DbErrorBadRequest('invalid lcr_route_sid');
+  }
+  const route = routes[0];
+
+  // Get the LCR and check ownership
+  const lcrs = await Lcr.retrieve(route.lcr_sid);
+  if (lcrs.length === 0) {
+    throw new DbErrorBadRequest('invalid lcr_sid');
+  }
+  const lcr = lcrs[0];
+
+  if (req.user.hasAdminAuth) return;
+
+  if (req.user.hasAccountAuth) {
+    if (!lcr.account_sid || lcr.account_sid !== req.user.account_sid) {
+      throw new DbErrorForbidden('insufficient privileges');
+    }
+  }
+
+  if (req.user.hasServiceProviderAuth) {
+    if (!lcr.service_provider_sid || lcr.service_provider_sid !== req.user.service_provider_sid) {
+      throw new DbErrorForbidden('insufficient privileges');
+    }
+  }
+};
+
 const preconditions = {
   add: validateAdd,
   update: validateUpdate,
+  retrieve: validateRetrieveOrDelete,
+  delete: validateRetrieveOrDelete,
 };
 
 decorate(router, LcrCarrierSetEntry, ['add', 'retrieve', 'update', 'delete'], preconditions);

lib/routes/api/lcr-routes.js

@@ -85,6 +85,7 @@ router.get('/:sid', async(req, res) => {
     const results = await LcrRoute.retrieve(lcr_route_sid);
     if (results.length === 0) return res.sendStatus(404);
     const route = results[0];
+    await checkUserScope(req, route.lcr_sid);
     route.lcr_carrier_set_entries = await LcrCarrierSetEntry.retrieveAllByLcrRouteSid(route.lcr_route_sid);
     res.status(200).json(route);
   } catch (err) {

lib/routes/api/sip-gateways.js

@@ -87,6 +87,7 @@ const validate = async(req, sid) => {
 
 const preconditions = {
   'add': validate,
+  'retrieve': validate,
   'update': validate,
   'delete': validate
 };

lib/routes/api/smpp-gateways.js

@@ -50,6 +50,7 @@ const validate = async(req, sid) => {
 
 const preconditions = {
   'add': validate,
+  'retrieve': validate,
   'update': validate,
   'delete': validate
 };

lib/routes/api/tenants.js

@@ -1,8 +1,84 @@
 const router = require('express').Router();
 const Tenant = require('../../models/tenant');
+const Account = require('../../models/account');
 const decorate = require('./decorate');
-const preconditions = {};
+const {DbErrorBadRequest, DbErrorForbidden} = require('../../utils/errors');
+const sysError = require('../error');
 
-decorate(router, Tenant, ['*'], preconditions);
+const checkTenantScope = async(req, tenant) => {
+  if (req.user.hasAdminAuth) return;
+
+  if (req.user.hasAccountAuth) {
+    if (tenant.account_sid !== req.user.account_sid) {
+      throw new DbErrorForbidden('insufficient privileges');
+    }
+  }
+
+  if (req.user.hasServiceProviderAuth) {
+    if (tenant.service_provider_sid !== req.user.service_provider_sid) {
+      throw new DbErrorForbidden('insufficient privileges');
+    }
+  }
+};
+
+const validateAdd = async(req) => {
+  if (req.user.hasAdminAuth) return;
+
+  const account_sid = req.body.account_sid;
+  if (!account_sid) {
+    throw new DbErrorBadRequest('missing account_sid');
+  }
+
+  if (req.user.hasAccountAuth) {
+    if (account_sid !== req.user.account_sid) {
+      throw new DbErrorForbidden('insufficient privileges');
+    }
+  }
+
+  if (req.user.hasServiceProviderAuth) {
+    const accounts = await Account.retrieve(account_sid, req.user.service_provider_sid);
+    if (accounts.length === 0) {
+      throw new DbErrorForbidden('insufficient privileges');
+    }
+  }
+};
+
+const validateRetrieveOrUpdateOrDelete = async(req, sid) => {
+  const tenants = await Tenant.retrieve(sid);
+  if (!tenants || tenants.length === 0) {
+    throw new DbErrorBadRequest('not found');
+  }
+  const tenant = tenants[0];
+  await checkTenantScope(req, tenant);
+};
+
+const preconditions = {
+  add: validateAdd,
+  retrieve: validateRetrieveOrUpdateOrDelete,
+  update: validateRetrieveOrUpdateOrDelete,
+  delete: validateRetrieveOrUpdateOrDelete,
+};
+
+decorate(router, Tenant, ['add', 'retrieve', 'update', 'delete'], preconditions);
+
+/* list - custom handler with proper scoping */
+router.get('/', async(req, res) => {
+  const logger = req.app.locals.logger;
+  try {
+    let results;
+    if (req.user.hasAdminAuth) {
+      results = await Tenant.retrieveAll();
+    } else if (req.user.hasAccountAuth) {
+      results = await Tenant.retrieveAll(req.user.account_sid);
+    } else if (req.user.hasServiceProviderAuth) {
+      results = await Tenant.retrieveAllByServiceProviderSid(req.user.service_provider_sid);
+    } else {
+      results = [];
+    }
+    res.status(200).json(results);
+  } catch (err) {
+    sysError(logger, res, err);
+  }
+});
 
 module.exports = router;