Wednesday, January 18, 2012

potential LinqSpecs security hole

The collection IdsOfManagedAllocationPrograms cannot be empty here or we'll get an NHibernate error:

public static Specification<Order> AllocationManagerCriteria(AccessMap accessMap)
{
   return new AdHocSpecification<Order>(o => o.OrderItems.Where(oi =>

         accessMap.IdsOfManagedAllocationPrograms.Contains(oi.ProgramOffering

         .ProgramPlan.Program.Id)).Count() > 0);
}

 
 

...so, we are trying to sanity check if we should run the spec like so:

using OurApp.Core.Entities;

using OurApp.Core.Infrastructure;

using OurApp.Core.Specs;

using OurApp.Core.Utils;

using OurApp.Core.Utils.LinqSpecifications;

namespace OurApp.Web.UI.LinqSpecifications.OrderSpecifications

{

   public class OrderSecuritySpecForAllocationManager :

         SecurityBasedSpecificationRuleBase<Order>,

         ISecurityBasedSpecificationRule<Order>

   {

      public override bool ShouldApply(UserContext userContext)

      {

         return userContext.AccessMap.IdsOfManagedAllocationPrograms.Count > 0;

      }

      

      public override SpecificationBuilder<Order> Apply(SpecificationBuilder<Order>

            specificationBuilder, UserContext userContext)

      {

         specificationBuilder.AddOr(OrderSpecs.AllocationManagerCriteria(userContext

               .AccessMap));

         return specificationBuilder;

      }

   }

}

 
 

We are aggregating applicable security specs and then running a query based upon those specs. We have to have at least one spec however (or face error) so, if at the end our of spec-building, if we have nothing to use in an IQueryable (due to none of the ShouldApply conditions being met) we use one of these specs.

using OurApp.Core.Entities;

using LinqSpecs;

namespace OurApp.Core.Specs

{

   public class BaseSpecs<T> where T : BaseEntity

   {

      public static Specification<T> Noop()

      {

         return new AdHocSpecification<T>(ssd => ssd != null);

      }

      

      public static Specification<T> EmptySet()

      {

         return new AdHocSpecification<T>(ssd => ssd == null);

      }

   }

}

 
 

We’ve been using Noop everywhere as the failover spec. I realized today that in security-filtering a list of objects that if may be possible to fall through all of the ShouldApply conditions and erroneously return an unfiltered list. It seems like the EmptySet spec is superior for anything that uses security based filtering.

No comments:

Post a Comment