[Suggestion] Apply operator whitelist option only for "external" provider by default #2445

Activity

1. jamesvillarrubia commented on Oct 19, 2021

   

   jamesvillarrubia

   on Oct 19, 2021

   Last edited by jamesvillarrubia

   Contributor

   More actions

   Can you post an example?

   If I understand your request, you could whitelist the entire set of query operators and then do a nested key search on the query object against a list of blocklist operators.

   // given this
   let params= {
      query: {
        $superSearch: "raw_string",
        createdAt: {
              $alt_lt: new Date().getTime() - 10000
        },
        $or: [
         { archived: { 
         	$ne: true,
            $loop: [
            	{yes:"no"}
            ]
          	} 
         },
         { roomId: 2 }
       ]
      }
   }
   
   // a quick function to get all the keys as an array
   const deepKeys = (obj) => {
     // get the keys
     return Object.keys(obj).reduce((res, el) => {
       if( Array.isArray(obj[el]) ) {
         return [...res, el, ...obj[el].map(deepKeys).flat()]
       } else if( typeof obj[el] === 'object' && obj[el] !== null ) {
         return [...res, el, ...deepKeys(obj[el])];
       }
       return [...res, el];
     }, []);
   }
   
   // then the hook is a simple check
   let hook = (options) => async (ctx) => {
     let keys = deepKeys(ctx.params.query)
     const filteredArray = options.blacklist.filter(value => keys.includes(value));
     if(filteredArray.length > 0){
       new BadRequest('Invalid Query Parameters', {
         errors: 'Cannot use query parameters: '+ filteredArray.join(" ")
       });
     }
   }
   
   hook({blacklist: ["$loop"]})({params})
2. matiaslopezd commented on Oct 19, 2021

   

   matiaslopezd

   on Oct 19, 2021

   Last edited by matiaslopezd

   Author

   More actions

   @jamesvillarrubia sure, thanks for your example, but it's not what I'm referencing, but maybe could help to create a PR.

   The current logic allows querying with whitelisted operators inclusive internally and externally, which is in fact a potential vulnerability.

   const { MyService } = require('./my-service.class');
   const createModel = require('../../models/my-service.model');
   const hooks = require('./my-service.hooks');
   
   module.exports = function (app) {
     const options = {
       Model: createModel(app),
       paginate: app.get('paginate'),
       whitelist: ['$populate', '$regex', '$options'] // <--- Need to add operators to whitelist to use it inclusive for internal query
     };
   
     // Initialize our service with any options it requires
     app.use('/my-service', new MyService(options, app));
   
     // Get our initialized service so that we can register hooks
     const service = app.service('my-service');
   
     service.hooks(hooks);
   };

   Proposal

   So, I'm proposing to apply the service whitelist only to external providers by default, which can get from context.provider.

   const options = {
       Model: createModel(app),
       paginate: app.get('paginate'),
       whitelist: ['$populate', '$regex', '$options'] // <--- Only apply to user request not internal query, ex: context.provider === 'external'
     };
   
   app.use('/my-service', new MyService(options, app));

   That's because don't have the sense to apply restrictions to the server-side querying.

   Whitelist as a function (extra proposal)

   Also, would be cool if can set a function as a value for the whitelist option, where also could pass the context argument like this:

   const options = {
       Model: createModel(app),
       paginate: app.get('paginate'),
       whitelist: context => {
         const { ip } = context.params;
         if (fromVPC(ip)) return ['$where'];
         else return ['$populate'];
       }
     };
   
   app.use('/my-service', new MyService(options, app));

   That will allow to customize different whitelist based on IP, or anything that comes in the context.
3. matiaslopezd

   changed the title [-]Bypass whitelisting rules for non-external "provider"[/-] [+][Suggestion] Apply operator whitelist option only for "external" provider by default[/+] on Oct 19, 2021
4. daffl

   closed this as completedin #2607on Apr 27, 2022