[Owasp-antisamy] dotNet - Some suggestions

Jerry Hoff jerry.hoff at aspectsecurity.com
Fri Mar 20 18:04:04 EDT 2009


Hi Steve,

Thank you for your suggestions. 

#1 - Agreed
#2 - Agreed
#3 - at first I didn't agree, but the more I thought about it, the more I like the idea. Would make it much faster, and better for situations where the config file doesn't change much.  If you'd like to contrib some code to that, i'd appreciate it.  Otherwise, I'll put it on my todo list.
#4 - Agreed

Thanks for the feedback! 

Jerry



-----Original Message-----
From: owasp-antisamy-bounces at lists.owasp.org on behalf of Jason Li
Sent: Sun 3/15/2009 11:30 PM
To: Steve Sheldon
Cc: owasp-antisamy at lists.owasp.org
Subject: Re: [Owasp-antisamy] dotNet - Some suggestions
 
Steve,

Thanks for the suggestions!

I'll let Jerry and Arshan speak to the .NET comments as I'm more familiar
with Java, but in terms of the XML-based policy file, I don't think that
will go away.

An XML-based policy file allows users to easily customize the policy without
recompiling code (which is extremely desirable). It also allows us maintain
a single white-list of HTML/CSS that can be used by both the Java and .NET
versions of AntiSamy whithout any additional work.

That's not to say that we can't come up with a programmatic way to modify
the policy. In fact, one of the many things we would like to do for future
versions of AntiSamy is to create a policy editor GUI. In doing so, it would
be a good design  to abstract out the functionality of creating/modifying
the policy so that the functionality can be exposed to anyone that wants to
access it programmatically. But even with this potential feature, I imagine
that the base policy behavior will remain XML-based.

--
-Jason Li-
-jason.li at owasp.org-


On Sun, Mar 15, 2009 at 2:55 PM, Steve Sheldon <steve at sodablue.org> wrote:

> I've been digging through the AntiSamy code, and have a couple of
> suggestions.
>
> 1. Make Action on Tag and OnInvalid on Attribute into enums.  Rather then
> doing string matches, I think it'd be better to have Enums... this would
> make it more clear what are acceptable values.
>
> 2. I would break up recursiveValidateTag into sub methods. for filter,
> validate and truncate actions.  Similarly validate should call sub methods
> for attribute validation.  The method is just too long and this would
> improve readibility.  Try to keep methods under 25 lines so they fit on the
> screen.  In combination with the enum in #1, it'd mean the use of a switch()
> instead of multiple if/then statements.
>
> 3. While I really like the use of Dependency Injection on the scanner with
> the Policy object, I'd suggest taking this one more step and use interfaces:
>
>   public interface IPolicy
>>   {
>>     string getDirective(string name);
>>     IAttribute getGlobalAttributeByName(string name);
>>     IProperty getPropertyByName(string propertyName);
>>     string getRegularExpression(string name);
>>     ITag getTagByName(string tagName);
>>   }
>
>
>   public interface IAttribute : ICloneable
>>   {
>>     IList AllowedValues { get; }
>>     IList AllowedRegExp { get; }
>>     string Description { get; set; }
>>     string Name { get; set; }
>>     string OnInvalid { get; set; }
>>   }
>>
>
>   public interface IProperty
>>   {
>>     string Description { get; set; }
>>     string Name { get; set; }
>>     string OnInvalid { get; set; }
>>   }
>>
>
>   public interface ITag
>>   {
>>     string Action { get; set; }
>>     Attribute getAttributeByName(string name);
>>     string Name { get; set; }
>>   }
>
>
> Using the interfaces would make it easier to extend the implementation of
> Policy with a custom implementation.
>
> Why? I'm just not a big fan of XML configuration.  So I am trying to
> implement a Fluent interface that looks something like this when used:
>
>     public static IPolicy GetFluentPolicy()
>>     {
>>         const string c_reghtmlTitle =
>> @"^([a-zA-Z0-9\s-_',:\[\]!\./\\\(\)]*)$";
>>         const string c_regonsiteURL = @"^^((\s)*((ht|f)tp(s?)://|mailto:
>> )[A-Za-z0-9]+[~a-zA-Z0-9-_\[email protected]#$%&;:,\?=/\+!]*(\s)*)$";
>>         const string c_regoffsiteURL = @"^((\s)*((ht|f)tp(s?)://|mailto:
>> )[A-Za-z0-9]+[~a-zA-Z0-9-_\[email protected]#$%&;:,\?=/\+!]*(\s)*)$";
>>
>>         FluentPolicy policy = new FluentPolicy()
>>           .AddDirective("maxInputSize", "500000")
>>           .AddCommonAttributes(new FluentAttribute("lang")
>>                                 .addAllowedRegExp(@"[a-zA-Z]{2,20}"))
>>           .AddCommonAttributes(new FluentAttribute("title")
>>                                 .addAllowedRegExp(c_reghtmlTitle))
>>           .AddCommonAttributes(new FluentAttribute("href")
>>                                 .addAllowedRegExp(c_regonsiteURL)
>>                                 .addAllowedRegExp(c_regoffsiteURL))
>>           .AddCommonAttributes(new FluentAttribute("align")
>>                                 .addAllowedValue("center")
>>                                 .addAllowedValue("left")
>>                                 .addAllowedValue("right")
>>                                 .addAllowedValue("justify")
>>                                 .addAllowedValue("char"))
>>           .AddGlobalAttribute("title")
>>           .AddGlobalAttribute("lang");
>>
>>       return policy;
>>     }
>
>
> While I'd probably keep an instance of IPolicy in my inversion of control
> container, I also suspect(but haven't verified) that instantiation of a
> policy using this interface would be faster than from an xml configuration
> file.
>
> 4. Also, I'd build AntiSamy as a class assembly, and place the unit tests
> and the console application logic in their own projects.  I just see no
> reason for them to be in the core implementation.
>
> I'll probably have some more thoughts as I dig in further.  It seems to me
> some of the logic in terms of global and tag specific interfaces could be
> better represented through the Policy class instead of having to be in the
> domscanner, as this is kind of logic specific to the policy... the notion
> that a tag attribute is implemented by tag specific attribute, or global
> attributes if they exist.
>
> Anyway, just some thoughts.
>
>
> _______________________________________________
> Owasp-antisamy mailing list
> Owasp-antisamy at lists.owasp.org
> https://lists.owasp.org/mailman/listinfo/owasp-antisamy
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://lists.owasp.org/pipermail/owasp-antisamy/attachments/20090320/1bcb3670/attachment.html 


More information about the Owasp-antisamy mailing list