[Seaside] Bug in WAAbstractFileLibrary

[Johan Brichau]

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 <jtuchel at objektfabrik.de> 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
> seaside at lists.squeakfoundation.org
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside