click-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bob Schellink <sab...@gmail.com>
Subject Re: Request Parameter Autobinding
Date Sat, 20 Nov 2010 13:35:11 GMT
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"

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.
> 
> 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?


> 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.

> 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

Mime
View raw message