You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Maarten Billemont (JIRA)" <ji...@apache.org> on 2010/03/25 21:23:27 UTC

[jira] Created: (WICKET-2801) User input can inject property model expressions using StringResourceModel

User input can inject property model expressions using StringResourceModel
--------------------------------------------------------------------------

                 Key: WICKET-2801
                 URL: https://issues.apache.org/jira/browse/WICKET-2801
             Project: Wicket
          Issue Type: Bug
          Components: wicket
    Affects Versions: 1.4.7
            Reporter: Maarten Billemont
            Priority: Critical


Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.

For instance, the following statement:

new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )

Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.

Consider the localization data:
key=User ${name} said: {0}

The user input:
input.getModelObject() = "My password is ${pass}."

The StringResourceModel's object would yield a string like:
User lhunath said: My password is secret

Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-2801) User input can inject property model expressions using StringResourceModel

Posted by "Jeremy Thomerson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2801?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850471#action_12850471 ] 

Jeremy Thomerson commented on WICKET-2801:
------------------------------------------

What Igor says is exactly right on this - you are using StringResourceModel to concatenate "system input" + "user input".  This is EXACTLY like concatenating "SELECT * FROM Users WHERE id = " + "user supplied ID".  One is from the application, one is from the user.  

This is simply NOT what StringResourceModel was created for.  See the javadocs - where it says clearly that StringResourceModel is for LOCALIZATION.  Then read what localization is: http://en.wikipedia.org/wiki/Language_localization - it's not the same as templating http://en.wikipedia.org/wiki/Template_(software_engineering) - combining application and user input into something to render.

As mentioned previously, you are absolutely free to create your own subclass that handles your use case.  But you can't force us to do this for you.  Wicket makes it super easy for you to extend these components and do it however you want or need to.



> User input can inject property model expressions using StringResourceModel
> --------------------------------------------------------------------------
>
>                 Key: WICKET-2801
>                 URL: https://issues.apache.org/jira/browse/WICKET-2801
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4.7
>            Reporter: Maarten Billemont
>            Assignee: Igor Vaynberg
>            Priority: Critical
>         Attachments: WICKET-2801-1.tbz2
>
>
> Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.
> For instance, the following statement:
> new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )
> Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.
> Consider the localization data:
> key=User ${name} said: {0}
> The user input:
> input.getModelObject() = "My password is ${pass}."
> The StringResourceModel's object would yield a string like:
> User lhunath said: My password is secret
> Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WICKET-2801) User input can inject property model expressions using StringResourceModel

Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-2801?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Maarten Billemont updated WICKET-2801:
--------------------------------------

    Attachment: WICKET-2801-1.tbz2

Test case which reproduces this issue.

> User input can inject property model expressions using StringResourceModel
> --------------------------------------------------------------------------
>
>                 Key: WICKET-2801
>                 URL: https://issues.apache.org/jira/browse/WICKET-2801
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4.7
>            Reporter: Maarten Billemont
>            Priority: Critical
>         Attachments: WICKET-2801-1.tbz2
>
>
> Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.
> For instance, the following statement:
> new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )
> Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.
> Consider the localization data:
> key=User ${name} said: {0}
> The user input:
> input.getModelObject() = "My password is ${pass}."
> The StringResourceModel's object would yield a string like:
> User lhunath said: My password is secret
> Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-2801) User input can inject property model expressions using StringResourceModel

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2801?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850019#action_12850019 ] 

Igor Vaynberg commented on WICKET-2801:
---------------------------------------

what next? should wicket also protect you against sql injection attacks by automatically escaping single quotes in case you decide to build a sql statement using string concatenations? you are responsible for sanitizing user input based on the context in which it will be used.

> User input can inject property model expressions using StringResourceModel
> --------------------------------------------------------------------------
>
>                 Key: WICKET-2801
>                 URL: https://issues.apache.org/jira/browse/WICKET-2801
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4.7
>            Reporter: Maarten Billemont
>            Assignee: Igor Vaynberg
>            Priority: Critical
>         Attachments: WICKET-2801-1.tbz2
>
>
> Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.
> For instance, the following statement:
> new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )
> Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.
> Consider the localization data:
> key=User ${name} said: {0}
> The user input:
> input.getModelObject() = "My password is ${pass}."
> The StringResourceModel's object would yield a string like:
> User lhunath said: My password is secret
> Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-2801) User input can inject property model expressions using StringResourceModel

Posted by "Jeremy Thomerson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2801?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850016#action_12850016 ] 

Jeremy Thomerson commented on WICKET-2801:
------------------------------------------

Yes - but what you are doing is not what StringResourceModel is intended to be used for.  You are saying "key=User ${name} said: {0} " where {0} is a piece of user-contributed data.  You should NOT be doing this.  The string resources are for internationalizing YOUR strings.  So, you should place just "key=User ${name} said:" in the StringResourceModel and the user-contributed String in a regular Label.

If your use case requires that you do anything other than what StringResourceModel was intended for, fine.  You can definitely accomplish what you need.  But the requirement of securing user data before using it in the StringResourceModel then becomes yours, not the framework's.

> User input can inject property model expressions using StringResourceModel
> --------------------------------------------------------------------------
>
>                 Key: WICKET-2801
>                 URL: https://issues.apache.org/jira/browse/WICKET-2801
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4.7
>            Reporter: Maarten Billemont
>            Assignee: Igor Vaynberg
>            Priority: Critical
>         Attachments: WICKET-2801-1.tbz2
>
>
> Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.
> For instance, the following statement:
> new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )
> Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.
> Consider the localization data:
> key=User ${name} said: {0}
> The user input:
> input.getModelObject() = "My password is ${pass}."
> The StringResourceModel's object would yield a string like:
> User lhunath said: My password is secret
> Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-2801) User input can inject property model expressions using StringResourceModel

Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2801?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850011#action_12850011 ] 

Maarten Billemont commented on WICKET-2801:
-------------------------------------------

What you are passing as arguments is definitely considered "data" rather than "application code".  In that light, it *must* remain untouched by any processing - whether "expected" or not.

Am I to understand that evaluating property expressions is a feature of the arguments?  Surely that cannot be the intended effect, for who would ever need their user data (not application code) to get evaluated as property expressions.  Conversely, I can think of a very many cases where one *needs* it not to be evaluated as such.

And if indeed it is to be considered "expected", then it should be very clearly noted as such in the documentation with a bold warning sign demanding users escape each of their arguments before passing them along - though truthfully, I do not expect even this will attract the necessary attention of all developers unknowingly dealing with this issue.

> User input can inject property model expressions using StringResourceModel
> --------------------------------------------------------------------------
>
>                 Key: WICKET-2801
>                 URL: https://issues.apache.org/jira/browse/WICKET-2801
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4.7
>            Reporter: Maarten Billemont
>            Assignee: Igor Vaynberg
>            Priority: Critical
>         Attachments: WICKET-2801-1.tbz2
>
>
> Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.
> For instance, the following statement:
> new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )
> Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.
> Consider the localization data:
> key=User ${name} said: {0}
> The user input:
> input.getModelObject() = "My password is ${pass}."
> The StringResourceModel's object would yield a string like:
> User lhunath said: My password is secret
> Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (WICKET-2801) User input can inject property model expressions using StringResourceModel

Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2801?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850011#action_12850011 ] 

Maarten Billemont edited comment on WICKET-2801 at 3/26/10 5:17 AM:
--------------------------------------------------------------------

What you are passing as arguments is definitely considered "data" rather than "application code".  In that light, it *must* remain untouched by any processing - whether "expected" or not.

Am I to understand that evaluating property expressions is a feature of the arguments?  Surely that cannot be the intended effect, for who would ever need their user data (not application code) to get evaluated as property expressions.  Conversely, I can think of a very many cases where one *needs* it not to be evaluated as such.

I believe the property evaluation string should be solely the value of the localization key, not the arguments given to it.

And if indeed it is to be considered "expected", then it should be very clearly noted as such in the documentation with a bold warning sign demanding users escape each of their arguments before passing them along - though truthfully, I do not expect even this will attract the necessary attention of all developers unknowingly dealing with this issue.

      was (Author: lhunath):
    What you are passing as arguments is definitely considered "data" rather than "application code".  In that light, it *must* remain untouched by any processing - whether "expected" or not.

Am I to understand that evaluating property expressions is a feature of the arguments?  Surely that cannot be the intended effect, for who would ever need their user data (not application code) to get evaluated as property expressions.  Conversely, I can think of a very many cases where one *needs* it not to be evaluated as such.

And if indeed it is to be considered "expected", then it should be very clearly noted as such in the documentation with a bold warning sign demanding users escape each of their arguments before passing them along - though truthfully, I do not expect even this will attract the necessary attention of all developers unknowingly dealing with this issue.
  
> User input can inject property model expressions using StringResourceModel
> --------------------------------------------------------------------------
>
>                 Key: WICKET-2801
>                 URL: https://issues.apache.org/jira/browse/WICKET-2801
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4.7
>            Reporter: Maarten Billemont
>            Assignee: Igor Vaynberg
>            Priority: Critical
>         Attachments: WICKET-2801-1.tbz2
>
>
> Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.
> For instance, the following statement:
> new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )
> Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.
> Consider the localization data:
> key=User ${name} said: {0}
> The user input:
> input.getModelObject() = "My password is ${pass}."
> The StringResourceModel's object would yield a string like:
> User lhunath said: My password is secret
> Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (WICKET-2801) User input can inject property model expressions using StringResourceModel

Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2801?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850175#action_12850175 ] 

Maarten Billemont edited comment on WICKET-2801 at 3/26/10 10:04 PM:
---------------------------------------------------------------------

I beg to differ, currently MessageFormat is being abused for something it is not intended for under the veil of uninformed "intent" of StringResourceModel.

Contrary to what you say, arguments to a format string are not application code; they should not contain any sort of syntax whatsoever.  Review MessageFormat's documentation for a description of its purpose which is clearly targeted at generating a string based off of a format string and data arguments that is to be displayed to the end-user.

I am very well aware that StringResourceModel *currently* considers its arguments as "application code" in the sense that it considers their contents to be of the same context as that of the localization value.  I also understand there are alternative safe methods of approaching this problem.  My argument, however, is against the sanity behind the existing API of StringResourceModel.  I am yet to learn of any argument *for* the way things are now, and I have given you plenty against - which I have not seen you dispute.  That alone should be enough to revert this bug to an Open state unless you can provide solid arguments as to why my impression of format arguments is so very wrong.

As for Igor's take on things; you seem to be missing the context of it all.  Contrary to what you say, my request is very much to NOT escape user data in any form while it resides within the application.  Escaping of data is not something that should be done by a developer as he introduces his data into a certain context - rather that context should be fit to take the data the developer offers him without allowing it to be evaluated as application code *by doing any necessary escaping itself, in ITS context and syntax*.
Similarly, "escaping single quotes in case you decide to build an SQL statement" is a fine example of introducing a form of "escaping" in application data.  It is as rediculus as you make it out to be since it would be escaping data outside of the context of what you're escaping it for.  At the same time, it is actually pretty close to what you're asking wicket developers to do now just before sending their data to StringResourceModel; escaping their own data outside of the context of property model expressions.
I think if you want to make an analogy; more fitting would be comparing this to new Label("id", "foo"); where foo is actually escaped by default by the framework.  As such; consistency dictates the developer can expect the same to happen for when he passes his "foo" to the framework as data elsewhere.

No, StringResourceModel should NOT allow or feature code injection, because the very purpose of format strings is to cleanly and safely introduce arguments into application code, rather than inject them into it.

      was (Author: lhunath):
    I beg to differ, currently MessageFormat is being abused for something it is not intended for under the veil of uninformed "intent" of StringResourceModel.

Contrary to what you say, arguments to a format string are not application code; they should not contain any sort of syntax whatsoever.  Review MessageFormat's documentation for a description of its purpose which is clearly targeted at generating a string based off of a format string and data arguments that is to be displayed to the end-user.

I am very well aware that StringResourceModel *currently* considers its arguments as "application code" in the sense that it considers their contents to be of the same context as that of the localization value.  I also understand there are alternative safe methods of approaching this problem.  My argument, however, is against the sanity behind the existing API of StringResourceModel.  I am yet to learn of any argument *for* the way things are now, and I have given you plenty against - which I have not seen you dispute.  That alone should be enough to revert this bug to an Open state unless you can provide solid arguments as to why my impression of format arguments is so very wrong.

As for Igor's take on things; you seem to be missing the context of it all.  Contrary to what you say, my request is very much to NOT escape user data in any form while it resides within the application.  Escaping of data is not something that should be done by a developer as he introduces his data into a certain context - rather that context should be fit to take the data the developer offers him without allowing it to be evaluated as application code *by doing any necessary escaping itself, in ITS context and syntax*.
Similarly, "escaping single quotes in case you decide to build an SQL statement" is a fine example of introducing a form of "escaping" in application data without even having it in the context of what the escaping is intended for.  That's quite ridiculous, as you make it out to be, and just as much beside the issue.
I think if you want to make an analogy; more fitting would be comparing this to new Label("id", "foo"); where foo is actually escaped by default by the framework.  As such; consistency dictates the developer can expect the same to happen for when he passes his "foo" to the framework as data elsewhere.

No, StringResourceModel should NOT allow or feature code injection, because the very purpose of format strings is to cleanly and safely introduce arguments into application code, rather than inject them into it.
  
> User input can inject property model expressions using StringResourceModel
> --------------------------------------------------------------------------
>
>                 Key: WICKET-2801
>                 URL: https://issues.apache.org/jira/browse/WICKET-2801
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4.7
>            Reporter: Maarten Billemont
>            Assignee: Igor Vaynberg
>            Priority: Critical
>         Attachments: WICKET-2801-1.tbz2
>
>
> Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.
> For instance, the following statement:
> new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )
> Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.
> Consider the localization data:
> key=User ${name} said: {0}
> The user input:
> input.getModelObject() = "My password is ${pass}."
> The StringResourceModel's object would yield a string like:
> User lhunath said: My password is secret
> Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-2801) User input can inject property model expressions using StringResourceModel

Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2801?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850175#action_12850175 ] 

Maarten Billemont commented on WICKET-2801:
-------------------------------------------

I beg to differ, currently MessageFormat is being abused for something it is not intended for under the veil of uninformed "intent" of StringResourceModel.

Contrary to what you say, arguments to a format string are not application code; they should not contain any sort of syntax whatsoever.  Review MessageFormat's documentation for a description of its purpose which is clearly targeted at generating a string based off of a format string and data arguments that is to be displayed to the end-user.

I am very well aware that StringResourceModel *currently* considers its arguments as "application code" in the sense that it considers their contents to be of the same context as that of the localization value.  My argument is against the sanity behind making that decision.  I am yet to learn of any argument *for* the way things are now, and I have given you plenty against - which I have not seen you dispute.  That alone should be enough to revert this bug to an Open state unless you can provide solid arguments as to why my impression of format arguments is so very wrong.

As for Igor's take on things; you seem to be missing the context of it all.  Contrary to what you say, my request is very much to NOT escape user data in any form while it resides within the application.  Escaping of data is not something that should be done by a developer as he introduces his data into a certain context - rather that context should be fit to take the data the developer offers him without allowing it to be evaluated as application code *by doing any necessary escaping itself, in ITS context and syntax*.
Similarly, "escaping single quotes in case you decide to build an SQL statement" is a fine example of introducing a form of "escaping" in application data without even having it in the context of what the escaping is intended for.  That's quite ridiculous, as you make it out to be, and just as much beside the issue.

No, StringResourceModel should NOT allow or feature code injection, because the very purpose of format strings is to cleanly and safely introduce arguments into application code, rather than inject them into it.

> User input can inject property model expressions using StringResourceModel
> --------------------------------------------------------------------------
>
>                 Key: WICKET-2801
>                 URL: https://issues.apache.org/jira/browse/WICKET-2801
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4.7
>            Reporter: Maarten Billemont
>            Assignee: Igor Vaynberg
>            Priority: Critical
>         Attachments: WICKET-2801-1.tbz2
>
>
> Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.
> For instance, the following statement:
> new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )
> Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.
> Consider the localization data:
> key=User ${name} said: {0}
> The user input:
> input.getModelObject() = "My password is ${pass}."
> The StringResourceModel's object would yield a string like:
> User lhunath said: My password is secret
> Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (WICKET-2801) User input can inject property model expressions using StringResourceModel

Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2801?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850175#action_12850175 ] 

Maarten Billemont edited comment on WICKET-2801 at 3/26/10 9:48 PM:
--------------------------------------------------------------------

I beg to differ, currently MessageFormat is being abused for something it is not intended for under the veil of uninformed "intent" of StringResourceModel.

Contrary to what you say, arguments to a format string are not application code; they should not contain any sort of syntax whatsoever.  Review MessageFormat's documentation for a description of its purpose which is clearly targeted at generating a string based off of a format string and data arguments that is to be displayed to the end-user.

I am very well aware that StringResourceModel *currently* considers its arguments as "application code" in the sense that it considers their contents to be of the same context as that of the localization value.  I also understand there are alternative safe methods of approaching this problem.  My argument, however, is against the sanity behind the existing API of StringResourceModel.  I am yet to learn of any argument *for* the way things are now, and I have given you plenty against - which I have not seen you dispute.  That alone should be enough to revert this bug to an Open state unless you can provide solid arguments as to why my impression of format arguments is so very wrong.

As for Igor's take on things; you seem to be missing the context of it all.  Contrary to what you say, my request is very much to NOT escape user data in any form while it resides within the application.  Escaping of data is not something that should be done by a developer as he introduces his data into a certain context - rather that context should be fit to take the data the developer offers him without allowing it to be evaluated as application code *by doing any necessary escaping itself, in ITS context and syntax*.
Similarly, "escaping single quotes in case you decide to build an SQL statement" is a fine example of introducing a form of "escaping" in application data without even having it in the context of what the escaping is intended for.  That's quite ridiculous, as you make it out to be, and just as much beside the issue.
I think if you want to make an analogy; more fitting would be comparing this to new Label("id", "foo"); where foo is actually escaped by default by the framework.  As such; consistency dictates the developer can expect the same to happen for when he passes his "foo" to the framework as data elsewhere.

No, StringResourceModel should NOT allow or feature code injection, because the very purpose of format strings is to cleanly and safely introduce arguments into application code, rather than inject them into it.

      was (Author: lhunath):
    I beg to differ, currently MessageFormat is being abused for something it is not intended for under the veil of uninformed "intent" of StringResourceModel.

Contrary to what you say, arguments to a format string are not application code; they should not contain any sort of syntax whatsoever.  Review MessageFormat's documentation for a description of its purpose which is clearly targeted at generating a string based off of a format string and data arguments that is to be displayed to the end-user.

I am very well aware that StringResourceModel *currently* considers its arguments as "application code" in the sense that it considers their contents to be of the same context as that of the localization value.  I also understand there are alternative safe methods of approaching this problem.  My argument, however, is against the sanity behind the existing API of StringResourceModel.  I am yet to learn of any argument *for* the way things are now, and I have given you plenty against - which I have not seen you dispute.  That alone should be enough to revert this bug to an Open state unless you can provide solid arguments as to why my impression of format arguments is so very wrong.

As for Igor's take on things; you seem to be missing the context of it all.  Contrary to what you say, my request is very much to NOT escape user data in any form while it resides within the application.  Escaping of data is not something that should be done by a developer as he introduces his data into a certain context - rather that context should be fit to take the data the developer offers him without allowing it to be evaluated as application code *by doing any necessary escaping itself, in ITS context and syntax*.
Similarly, "escaping single quotes in case you decide to build an SQL statement" is a fine example of introducing a form of "escaping" in application data without even having it in the context of what the escaping is intended for.  That's quite ridiculous, as you make it out to be, and just as much beside the issue.

No, StringResourceModel should NOT allow or feature code injection, because the very purpose of format strings is to cleanly and safely introduce arguments into application code, rather than inject them into it.
  
> User input can inject property model expressions using StringResourceModel
> --------------------------------------------------------------------------
>
>                 Key: WICKET-2801
>                 URL: https://issues.apache.org/jira/browse/WICKET-2801
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4.7
>            Reporter: Maarten Billemont
>            Assignee: Igor Vaynberg
>            Priority: Critical
>         Attachments: WICKET-2801-1.tbz2
>
>
> Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.
> For instance, the following statement:
> new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )
> Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.
> Consider the localization data:
> key=User ${name} said: {0}
> The user input:
> input.getModelObject() = "My password is ${pass}."
> The StringResourceModel's object would yield a string like:
> User lhunath said: My password is secret
> Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (WICKET-2801) User input can inject property model expressions using StringResourceModel

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-2801?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Igor Vaynberg resolved WICKET-2801.
-----------------------------------

    Resolution: Invalid
      Assignee: Igor Vaynberg

you choose to wire user's input directly to a property evaluator, in which case what you see is the expected result. if you dont want it to happen then wrap the model in another that escapes the $ character with wicket's escape sequence $$

> User input can inject property model expressions using StringResourceModel
> --------------------------------------------------------------------------
>
>                 Key: WICKET-2801
>                 URL: https://issues.apache.org/jira/browse/WICKET-2801
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4.7
>            Reporter: Maarten Billemont
>            Assignee: Igor Vaynberg
>            Priority: Critical
>         Attachments: WICKET-2801-1.tbz2
>
>
> Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.
> For instance, the following statement:
> new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )
> Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.
> Consider the localization data:
> key=User ${name} said: {0}
> The user input:
> input.getModelObject() = "My password is ${pass}."
> The StringResourceModel's object would yield a string like:
> User lhunath said: My password is secret
> Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (WICKET-2801) User input can inject property model expressions using StringResourceModel

Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2801?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850175#action_12850175 ] 

Maarten Billemont edited comment on WICKET-2801 at 3/26/10 3:19 PM:
--------------------------------------------------------------------

I beg to differ, currently MessageFormat is being abused for something it is not intended for under the veil of uninformed "intent" of StringResourceModel.

Contrary to what you say, arguments to a format string are not application code; they should not contain any sort of syntax whatsoever.  Review MessageFormat's documentation for a description of its purpose which is clearly targeted at generating a string based off of a format string and data arguments that is to be displayed to the end-user.

I am very well aware that StringResourceModel *currently* considers its arguments as "application code" in the sense that it considers their contents to be of the same context as that of the localization value.  I also understand there are alternative safe methods of approaching this problem.  My argument, however, is against the sanity behind the existing API of StringResourceModel.  I am yet to learn of any argument *for* the way things are now, and I have given you plenty against - which I have not seen you dispute.  That alone should be enough to revert this bug to an Open state unless you can provide solid arguments as to why my impression of format arguments is so very wrong.

As for Igor's take on things; you seem to be missing the context of it all.  Contrary to what you say, my request is very much to NOT escape user data in any form while it resides within the application.  Escaping of data is not something that should be done by a developer as he introduces his data into a certain context - rather that context should be fit to take the data the developer offers him without allowing it to be evaluated as application code *by doing any necessary escaping itself, in ITS context and syntax*.
Similarly, "escaping single quotes in case you decide to build an SQL statement" is a fine example of introducing a form of "escaping" in application data without even having it in the context of what the escaping is intended for.  That's quite ridiculous, as you make it out to be, and just as much beside the issue.

No, StringResourceModel should NOT allow or feature code injection, because the very purpose of format strings is to cleanly and safely introduce arguments into application code, rather than inject them into it.

      was (Author: lhunath):
    I beg to differ, currently MessageFormat is being abused for something it is not intended for under the veil of uninformed "intent" of StringResourceModel.

Contrary to what you say, arguments to a format string are not application code; they should not contain any sort of syntax whatsoever.  Review MessageFormat's documentation for a description of its purpose which is clearly targeted at generating a string based off of a format string and data arguments that is to be displayed to the end-user.

I am very well aware that StringResourceModel *currently* considers its arguments as "application code" in the sense that it considers their contents to be of the same context as that of the localization value.  My argument is against the sanity behind making that decision.  I am yet to learn of any argument *for* the way things are now, and I have given you plenty against - which I have not seen you dispute.  That alone should be enough to revert this bug to an Open state unless you can provide solid arguments as to why my impression of format arguments is so very wrong.

As for Igor's take on things; you seem to be missing the context of it all.  Contrary to what you say, my request is very much to NOT escape user data in any form while it resides within the application.  Escaping of data is not something that should be done by a developer as he introduces his data into a certain context - rather that context should be fit to take the data the developer offers him without allowing it to be evaluated as application code *by doing any necessary escaping itself, in ITS context and syntax*.
Similarly, "escaping single quotes in case you decide to build an SQL statement" is a fine example of introducing a form of "escaping" in application data without even having it in the context of what the escaping is intended for.  That's quite ridiculous, as you make it out to be, and just as much beside the issue.

No, StringResourceModel should NOT allow or feature code injection, because the very purpose of format strings is to cleanly and safely introduce arguments into application code, rather than inject them into it.
  
> User input can inject property model expressions using StringResourceModel
> --------------------------------------------------------------------------
>
>                 Key: WICKET-2801
>                 URL: https://issues.apache.org/jira/browse/WICKET-2801
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4.7
>            Reporter: Maarten Billemont
>            Assignee: Igor Vaynberg
>            Priority: Critical
>         Attachments: WICKET-2801-1.tbz2
>
>
> Applications that use StringResourceModel to render localized strings using a model and value arguments are subject to a security issue which allows users to perform property model expressions on the given model.
> For instance, the following statement:
> new StringResourceModel( "key", userModel, new Object[] { input.getModelObject() } )
> Would expand property model expressions from input's object against userModel's object, effectively allowing users to access unintended data from userModel's object.
> Consider the localization data:
> key=User ${name} said: {0}
> The user input:
> input.getModelObject() = "My password is ${pass}."
> The StringResourceModel's object would yield a string like:
> User lhunath said: My password is secret
> Find attached test case which illustrates this problem using WicketTester.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.