On Nov 20, 2010, at 14:35 , Bob Schellink wrote:
> Hi,
>
>
> On 20/11/2010 22:43, Lorenzo Simionato wrote:
>>
>> 1-Is it possible to disable the feature that: "binds automatically any
>> request parameter values to public Page fields with the same name"?
>
>
> Yep, just set autobinding="none"
OK. From the user guide it seemed that autobinding="none" was only for Page
Autobinding and not for Request Parameters.
>
> Binding request parameters will only be done for "public" or Bindable fields.
> Java devs generally
> delare fields as private or protected so unlikely to be an issue.
>
>> I see that it is possible for page autobinding but not for request
>> parameters. I find this feature very subtle, makes the code less clear and
>> is
>
> I don't like the feature much either. One thing I'd like to do for a future
> release is revamping the
> click-examples to not use autobinding.
>
>> possibly dangerous (class fields can be set by an attacker in a way that is
>> not evident and it is easy to make mistakes).
>
> If you declare the field as @Bindable how is it more dangerous than using a
> Form or link? An
> attacker can tamper with any value.
Here what i meant is that if you declare a field as @Bindable you are clearly
aware that it can be set in some way by the user.
If you have a public field (ok, it's rare) this is not that obvious.
>>
>> 2-According to the documentation: "When binding these values Click will also
>> attempt to convert them to the correct type". However, if the
>> conversion is not successful is the intended behavior to throw an exception?
>> Say i have a page:
>> public class MyPage extends Page {
>> @Bindable
>> protected Integer customerId;
>> }
>> and the following request is made: mypage.htm?customerId=xxx
>>
>> In this case an exception is thrown.
>
> Why is this a problem? If your an attacker is tampering with data what should
> your application do?
Maybe set the field to null, but the exception is okay. Just asking if this was
the indented behavior.
>
>
>> 3-Why the @Bindable annotation is used both for request parameters and for
>> page autobinding (if autobinding for pages is enabled of course)?
>> This makes it very confusing. It is not clear if @Bindable is used to get a
>> parameter or put something on the page.
>
> Yeah I don't like it either. Ideally Bindable should only work for request
> parameters not Controls.
> However this will be difficult to add as majority of existing apps will
> break. First thing is to
> revamp the examples so people aren't encouraged to use this pattern.
A possibility could be to introduce a new annotation like @InputParameter or
something like this to clearly separate the two things.
Backward compatibility can be possibly maintained by adding an option into
click.xml (e.g. a new value for the aubinding property)
>
>> In addition, it could lead to security problems.
>> For example, consider the page:
>> MyPage.java:
>> public class MyPage extends Page {
>> @Bindable
>> protected String welcomeMessage = "Welcome to my web site";
>
>
> The security problem doesn't come from Bindable but from rendering without
> escaping. The Velocity
> template model can be populated from anywhere including forms, arbitrary
> request params, web
> services, external systems etc. For example:
>
> addModel("msg", getContext().getRequestParameter("welcomeMessage"));
>
> If you are rendering output that could be dangerous you should escape it:
>
> $format.html($msg);
>
> In our app we've gone even further, specifying a Velocity
> ReferenceInsertionEventHandler.
>
>
> eventhandler.referenceinsertion.class=org.apache.velocity.app.event.implement.EscapeHtmlReference
> eventhandler.escape.html.match = /_.*/
>
> So any variable in the template that starts with _ is escaped. In our border
> template we overrode
> addModel to ensure all non control objects added are prefixed with an
> underscore. Since Controls
> escape their values and attributes we had to exclude them otherwise they are
> doubly escaped. The net
> effect is that when people rendered Velocity model objects they had to use:
>
> $_customer
>
> Not that user-friendly, but safe.
>
> Kind regards
>
> Bob
Here the XSS was just an example. The fact that one can set a value that i
intended only for output is disturbing.
As a couple of other examples:
-suppose the welcomeMessage is the title of the page. It's not nice that one
can put an arbitrary title on the page, even if it is escaped properly.
-suppose one modifies the RequestTypeConverter as explained in the
documentation to dynamically load customers from the db.
In a page one would like to do something with a customer object and then print
the details, so we could have something like:
MyPage.class
public class MyPage extends Page {
@Bindable protected Customer customer = loadCustomer(3);
pubic void action() {
customer.set.....();
}
MyPage.html
$customer.name
a different customer can be loaded with a request like mypage.htm?customer=56
(this example is a little weird but is just to get the idea)
These are just examples and maybe if all is handled very carefully by the
programmer there would not be any problems.
However, they demonstrate that it is easy to make something that does not work
as intended.
As a last example consider SQL Injections: if you escape the input properly you
do not have the problem. On the other hand,
to prevent the problem even if you are not that careful PHP has introduced
magic quotes and in Java we have preparedStatements (yes i'm simplifying a lot
the things here!). The concept is that this double role of public field and
ones annotated with @Bindable (as parameters and variables added to the page)
it does not seem a good idea to me.
--
Lorenzo
