Bug in WAAbstractFileLibrary

> Hi there,
>
> over the last few nights, our Seaside Application was bombarded with
> requests that were formed like this:
>
> /files/JQUiDeploymentLibrary/%29.find%28
>
> The attacks did also try other javascript expressions.
>
> Unfortunately, WAAbstractFileLibrary reacts to this by throwing a primitive
> failed on VA Smalltalk in WAAbstractFileLibrary class>>#asSelector:, because
> the javascript expression cannot be interpreted as a filename.
>
> Here's an excerpt of our walkback that shows what's going on.
>
> String(Object)>>#primitiveFailed
>   receiver = ''
> String>>#at:
>   receiver = ''
>   arg1 = 1
> String(SequenceableCollection)>>#first
>   receiver = ''
> JQUiDeploymentLibrary class(WAAbstractFileLibrary class)>>#asSelector:
>   receiver = JQUiDeploymentLibrary
>   arg1 = ').find('
>   temp1 = ''
>   temp2 = nil
> JQUiDeploymentLibrary(WAAbstractFileLibrary)>>#asSelector:
>   receiver = a JQUiDeploymentLibrary
>   arg1 = ').find('
> JQUiDeploymentLibrary(WAFileLibrary)>>#handle:
>   receiver = a JQUiDeploymentLibrary
>   arg1 = a WARequestContext url: '/files/JQUiDeploymentLibrary/%29.find%28'
>   temp1 = ').find('
>   temp2 = nil
>   temp3 = nil
> JQUiDeploymentLibrary class(WAAbstractFileLibrary class)>>#handle:
>   receiver = JQUiDeploymentLibrary
>   arg1 = a WARequestContext url: '/files/JQUiDeploymentLibrary/%29.find%28'
>
> I am on the road and have no pharo/seaside image with me, but if I remember
> correctly, pharo does not throw an exception when you ask an empty string
> for its #first character, I seem to remember it just returns nil. VA
> Smalltalk does throw an exception. It does not stop working, so this is not
> a critical problem.
>
> However, I think an additional check in #asSelector: wouldn't hurt because
> then the result is an http error code 404, which can either be returned to
> the client or removed by filters like mod_security.
>
> So here is a fix for WAAbstractFileLibrary class>>asSelector: that I suggest
> for inclusion in Seaside, even if it is unnecessary for Pharo:
>
> asSelector: aFilename
>     | mainPart extension |
>     mainPart := (aFilename copyUpToLast: $.)
>         select: [ :each | each isAlphaNumeric ].
>
>     mainPart isEmptyOrNil ifTrue: [^nil].
>
>     [ mainPart first isDigit ]
>         whileTrue: [ mainPart := mainPart allButFirst ].
>     extension := (aFilename copyAfterLast: $.) asLowercase capitalized.
>     ^ (mainPart, extension) asSymbol

> Hi there,
>
> over the last few nights, our Seaside Application was bombarded with requests that were formed like this:
>
> /files/JQUiDeploymentLibrary/%29.find%28
>
> The attacks did also try other javascript expressions.
>
> Unfortunately, WAAbstractFileLibrary reacts to this by throwing a primitive failed on VA Smalltalk in WAAbstractFileLibrary class>>#asSelector:, because the javascript expression cannot be interpreted as a filename.
>
> Here's an excerpt of our walkback that shows what's going on.
>
> String(Object)>>#primitiveFailed
>  receiver = ''
> String>>#at:
>  receiver = ''
>  arg1 = 1
> String(SequenceableCollection)>>#first
>  receiver = ''
> JQUiDeploymentLibrary class(WAAbstractFileLibrary class)>>#asSelector:
>  receiver = JQUiDeploymentLibrary
>  arg1 = ').find('
>  temp1 = ''
>  temp2 = nil
> JQUiDeploymentLibrary(WAAbstractFileLibrary)>>#asSelector:
>  receiver = a JQUiDeploymentLibrary
>  arg1 = ').find('
> JQUiDeploymentLibrary(WAFileLibrary)>>#handle:
>  receiver = a JQUiDeploymentLibrary
>  arg1 = a WARequestContext url: '/files/JQUiDeploymentLibrary/%29.find%28'
>  temp1 = ').find('
>  temp2 = nil
>  temp3 = nil
> JQUiDeploymentLibrary class(WAAbstractFileLibrary class)>>#handle:
>  receiver = JQUiDeploymentLibrary
>  arg1 = a WARequestContext url: '/files/JQUiDeploymentLibrary/%29.find%28'
>
> I am on the road and have no pharo/seaside image with me, but if I remember correctly, pharo does not throw an exception when you ask an empty string for its #first character, I seem to remember it just returns nil. VA Smalltalk does throw an exception. It does not stop working, so this is not a critical problem.
>
> However, I think an additional check in #asSelector: wouldn't hurt because then the result is an http error code 404, which can either be returned to the client or removed by filters like mod_security.
>
> So here is a fix for WAAbstractFileLibrary class>>asSelector: that I suggest for inclusion in Seaside, even if it is unnecessary for Pharo:
>
> asSelector: aFilename
>    | mainPart extension |
>    mainPart := (aFilename copyUpToLast: $.)
>        select: [ :each | each isAlphaNumeric ].
>
>    mainPart isEmptyOrNil ifTrue: [^nil].
>
>    [ mainPart first isDigit ]
>        whileTrue: [ mainPart := mainPart allButFirst ].
>    extension := (aFilename copyAfterLast: $.) asLowercase capitalized.
>    ^ (mainPart, extension) asSymbol
>
> Joachim
>
>
>
> _______________________________________________
> seaside mailing list
> [hidden email]
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside

> On Mon, Apr 14, 2014 at 6:01 AM, Joachim Tuchel <[hidden email]> wrote:
>> Hi there,
>>
>> over the last few nights, our Seaside Application was bombarded with
>> requests that were formed like this:
>>
>> /files/JQUiDeploymentLibrary/%29.find%28
>>
>> The attacks did also try other javascript expressions.
>>
>> Unfortunately, WAAbstractFileLibrary reacts to this by throwing a primitive
>> failed on VA Smalltalk in WAAbstractFileLibrary class>>#asSelector:, because
>> the javascript expression cannot be interpreted as a filename.
>>
>> Here's an excerpt of our walkback that shows what's going on.
>>
>> String(Object)>>#primitiveFailed
>>  receiver = ''
>> String>>#at:
>>  receiver = ''
>>  arg1 = 1
>> String(SequenceableCollection)>>#first
>>  receiver = ''
>> JQUiDeploymentLibrary class(WAAbstractFileLibrary class)>>#asSelector:
>>  receiver = JQUiDeploymentLibrary
>>  arg1 = ').find('
>>  temp1 = ''
>>  temp2 = nil
>> JQUiDeploymentLibrary(WAAbstractFileLibrary)>>#asSelector:
>>  receiver = a JQUiDeploymentLibrary
>>  arg1 = ').find('
>> JQUiDeploymentLibrary(WAFileLibrary)>>#handle:
>>  receiver = a JQUiDeploymentLibrary
>>  arg1 = a WARequestContext url: '/files/JQUiDeploymentLibrary/%29.find%28'
>>  temp1 = ').find('
>>  temp2 = nil
>>  temp3 = nil
>> JQUiDeploymentLibrary class(WAAbstractFileLibrary class)>>#handle:
>>  receiver = JQUiDeploymentLibrary
>>  arg1 = a WARequestContext url: '/files/JQUiDeploymentLibrary/%29.find%28'
>>
>> I am on the road and have no pharo/seaside image with me, but if I remember
>> correctly, pharo does not throw an exception when you ask an empty string
>> for its #first character, I seem to remember it just returns nil. VA
>> Smalltalk does throw an exception. It does not stop working, so this is not
>> a critical problem.
>>
>> However, I think an additional check in #asSelector: wouldn't hurt because
>> then the result is an http error code 404, which can either be returned to
>> the client or removed by filters like mod_security.
>>
>> So here is a fix for WAAbstractFileLibrary class>>asSelector: that I suggest
>> for inclusion in Seaside, even if it is unnecessary for Pharo:
>>
>> asSelector: aFilename
>>    | mainPart extension |
>>    mainPart := (aFilename copyUpToLast: $.)
>>        select: [ :each | each isAlphaNumeric ].
>>
>>    mainPart isEmptyOrNil ifTrue: [^nil].
>>
>>    [ mainPart first isDigit ]
>>        whileTrue: [ mainPart := mainPart allButFirst ].
>>    extension := (aFilename copyAfterLast: $.) asLowercase capitalized.
>>    ^ (mainPart, extension) asSymbol
>
> https://code.google.com/p/seaside/issues/detail?id=786
>
> Cheers
> Philippe
> _______________________________________________
> seaside mailing list
> [hidden email]
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>

> Hi Joachim,
>
> Thank you for the report and the suggested fix.
> I think that returning a 404 rather than throwing an exception is indeed a good suggestion.
> In Pharo 3, at least, this also throws an exception (it is not ignored), so the behaviour is the same.
>
> But most importantly, I think you should not use a WAFileLibrary in production.
> Not really because of these problems, but because serving static files can be done way more efficiently by your front-end web server.
>
> best regards,
> Johan
>
> On 14 Apr 2014, at 06:01, Joachim Tuchel <[hidden email]> wrote:
>
>> Hi there,
>>
>> over the last few nights, our Seaside Application was bombarded with requests that were formed like this:
>>
>> /files/JQUiDeploymentLibrary/%29.find%28
>>
>> The attacks did also try other javascript expressions.
>>
>> Unfortunately, WAAbstractFileLibrary reacts to this by throwing a primitive failed on VA Smalltalk in WAAbstractFileLibrary class>>#asSelector:, because the javascript expression cannot be interpreted as a filename.
>>
>> Here's an excerpt of our walkback that shows what's going on.
>>
>> String(Object)>>#primitiveFailed
>>   receiver = ''
>> String>>#at:
>>   receiver = ''
>>   arg1 = 1
>> String(SequenceableCollection)>>#first
>>   receiver = ''
>> JQUiDeploymentLibrary class(WAAbstractFileLibrary class)>>#asSelector:
>>   receiver = JQUiDeploymentLibrary
>>   arg1 = ').find('
>>   temp1 = ''
>>   temp2 = nil
>> JQUiDeploymentLibrary(WAAbstractFileLibrary)>>#asSelector:
>>   receiver = a JQUiDeploymentLibrary
>>   arg1 = ').find('
>> JQUiDeploymentLibrary(WAFileLibrary)>>#handle:
>>   receiver = a JQUiDeploymentLibrary
>>   arg1 = a WARequestContext url: '/files/JQUiDeploymentLibrary/%29.find%28'
>>   temp1 = ').find('
>>   temp2 = nil
>>   temp3 = nil
>> JQUiDeploymentLibrary class(WAAbstractFileLibrary class)>>#handle:
>>   receiver = JQUiDeploymentLibrary
>>   arg1 = a WARequestContext url: '/files/JQUiDeploymentLibrary/%29.find%28'
>>
>> I am on the road and have no pharo/seaside image with me, but if I remember correctly, pharo does not throw an exception when you ask an empty string for its #first character, I seem to remember it just returns nil. VA Smalltalk does throw an exception. It does not stop working, so this is not a critical problem.
>>
>> However, I think an additional check in #asSelector: wouldn't hurt because then the result is an http error code 404, which can either be returned to the client or removed by filters like mod_security.
>>
>> So here is a fix for WAAbstractFileLibrary class>>asSelector: that I suggest for inclusion in Seaside, even if it is unnecessary for Pharo:
>>
>> asSelector: aFilename
>>     | mainPart extension |
>>     mainPart := (aFilename copyUpToLast: $.)
>>         select: [ :each | each isAlphaNumeric ].
>>
>>     mainPart isEmptyOrNil ifTrue: [^nil].
>>
>>     [ mainPart first isDigit ]
>>         whileTrue: [ mainPart := mainPart allButFirst ].
>>     extension := (aFilename copyAfterLast: $.) asLowercase capitalized.
>>     ^ (mainPart, extension) asSymbol
>>
>> Joachim
>>
>>
>>
>> _______________________________________________
>> seaside mailing list
>> [hidden email]
>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
> _______________________________________________
> seaside mailing list
> [hidden email]
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>