You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@wicket.apache.org by Daniel Stoch <da...@gmail.com> on 2020/05/25 18:47:36 UTC

FilenameWithVersionResourceCachingStrategy accepts (almost) any string as a version

Hi,

Each resource in Wicket is decorated using a version string in a file
name by default. It is implemented in
FilenameWithVersionResourceCachingStrategy. Depending on DEVELOPMENT
or DEPLOYMENT mode it looks like:
jquery-ver-1590158412000.css
jquery-ver-F334A4E46CB37347CAB42E2B1A45897C.css

There is a small security issue, that this strategy does not check if
this version is correctly calculated for specific resource and accepts
any string as a version identifier, eg.:
jquery-ver-F334A4E46CB37347CAB42E2B1A45897C_old.css
jquery-ver-F334A4E46CB37347CAB42E2B1A45897C_bakup.css
jquery-ver-XYZABCDEF.css
etc.

Maybe we should add a check if version passed in request is correct?
There is partially such check done in decorateResponse() method. So
maybe it is enough to add else block here and raise some exception?

@Override
public void decorateResponse(AbstractResource.ResourceResponse
response, IStaticCacheableResource resource) {
  String requestedVersion = RequestCycle.get().getMetaData(URL_VERSION);
  String calculatedVersion = this.resourceVersion.getVersion(resource);
  if (calculatedVersion != null && calculatedVersion.equals(requestedVersion)) {
    response.setCacheDurationToMaximum();
    response.setCacheScope(WebResponse.CacheScope.PUBLIC);
  }
}

--
Best regards,
Daniel Stoch

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org


Re: FilenameWithVersionResourceCachingStrategy accepts (almost) any string as a version

Posted by Martin Grigorov <mg...@apache.org>.
On Tue, May 26, 2020 at 11:59 AM Daniel Stoch <da...@gmail.com>
wrote:

> I know that and for me this is not an issue either ;).
> But this "issue" is reported by some security scanners which checks
> for obsolete and backup files by adding "_old", "_bak", "_backup"
> suffix to filename of selected resource (css, js). And our Wicket
> application is serving such files as if indeed such old copies were
> available.
> So I'm only loudly thinking is it really no problem that we serve
> files with any text attached on the end of file name.
>

This is by design.
If it is not desired then you can use a custom strategy.


>
> --
> Daniel
>
>
> pon., 25 maj 2020 o 21:14 Carl-Eric Menzel <cm...@wicketbuch.de>
> napisał(a):
> >
> > Sorry, didn't mean to sound dismissive. It's a valid point, just I'm
> > not seeing that anybody could get to anything otherwise unavailable.
> >
> > On Mon, 25 May 2020 21:02:08 +0200
> > Carl-Eric Menzel <cm...@wicketbuch.de> wrote:
> >
> > > I think the point of this version decoration is not to ensure a
> > > particular version is requested, because typically only one version of
> > > a file is available in the application.
> > >
> > > The point is instead to defeat any caching, both in the browser and by
> > > proxies, which might serve the user an outdated version. So I don't
> > > think there needs to be any checking of that string.
> > >
> > > Or is there an actual security impact that I'm missing?
> > >
> > > Carl-Eric
> > >
> > > On Mon, 25 May 2020 20:47:36 +0200
> > > Daniel Stoch <da...@gmail.com> wrote:
> > >
> > > > Hi,
> > > >
> > > > Each resource in Wicket is decorated using a version string in a
> > > > file name by default. It is implemented in
> > > > FilenameWithVersionResourceCachingStrategy. Depending on DEVELOPMENT
> > > > or DEPLOYMENT mode it looks like:
> > > > jquery-ver-1590158412000.css
> > > > jquery-ver-F334A4E46CB37347CAB42E2B1A45897C.css
> > > >
> > > > There is a small security issue, that this strategy does not check
> > > > if this version is correctly calculated for specific resource and
> > > > accepts any string as a version identifier, eg.:
> > > > jquery-ver-F334A4E46CB37347CAB42E2B1A45897C_old.css
> > > > jquery-ver-F334A4E46CB37347CAB42E2B1A45897C_bakup.css
> > > > jquery-ver-XYZABCDEF.css
> > > > etc.
> > > >
> > > > Maybe we should add a check if version passed in request is correct?
> > > > There is partially such check done in decorateResponse() method. So
> > > > maybe it is enough to add else block here and raise some exception?
> > > >
> > > > @Override
> > > > public void decorateResponse(AbstractResource.ResourceResponse
> > > > response, IStaticCacheableResource resource) {
> > > >   String requestedVersion =
> > > > RequestCycle.get().getMetaData(URL_VERSION); String
> > > > calculatedVersion = this.resourceVersion.getVersion(resource); if
> > > > (calculatedVersion != null &&
> > > > calculatedVersion.equals(requestedVersion))
> > > > { response.setCacheDurationToMaximum();
> > > > response.setCacheScope(WebResponse.CacheScope.PUBLIC); } }
> > > >
> > > > --
> > > > Best regards,
> > > > Daniel Stoch
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> > > > For additional commands, e-mail: users-help@wicket.apache.org
> > > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> > > For additional commands, e-mail: users-help@wicket.apache.org
> > >
> >
> > 000
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> > For additional commands, e-mail: users-help@wicket.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
>
>

Re: FilenameWithVersionResourceCachingStrategy accepts (almost) any string as a version

Posted by Daniel Stoch <da...@gmail.com>.
I know that and for me this is not an issue either ;).
But this "issue" is reported by some security scanners which checks
for obsolete and backup files by adding "_old", "_bak", "_backup"
suffix to filename of selected resource (css, js). And our Wicket
application is serving such files as if indeed such old copies were
available.
So I'm only loudly thinking is it really no problem that we serve
files with any text attached on the end of file name.

--
Daniel


pon., 25 maj 2020 o 21:14 Carl-Eric Menzel <cm...@wicketbuch.de> napisał(a):
>
> Sorry, didn't mean to sound dismissive. It's a valid point, just I'm
> not seeing that anybody could get to anything otherwise unavailable.
>
> On Mon, 25 May 2020 21:02:08 +0200
> Carl-Eric Menzel <cm...@wicketbuch.de> wrote:
>
> > I think the point of this version decoration is not to ensure a
> > particular version is requested, because typically only one version of
> > a file is available in the application.
> >
> > The point is instead to defeat any caching, both in the browser and by
> > proxies, which might serve the user an outdated version. So I don't
> > think there needs to be any checking of that string.
> >
> > Or is there an actual security impact that I'm missing?
> >
> > Carl-Eric
> >
> > On Mon, 25 May 2020 20:47:36 +0200
> > Daniel Stoch <da...@gmail.com> wrote:
> >
> > > Hi,
> > >
> > > Each resource in Wicket is decorated using a version string in a
> > > file name by default. It is implemented in
> > > FilenameWithVersionResourceCachingStrategy. Depending on DEVELOPMENT
> > > or DEPLOYMENT mode it looks like:
> > > jquery-ver-1590158412000.css
> > > jquery-ver-F334A4E46CB37347CAB42E2B1A45897C.css
> > >
> > > There is a small security issue, that this strategy does not check
> > > if this version is correctly calculated for specific resource and
> > > accepts any string as a version identifier, eg.:
> > > jquery-ver-F334A4E46CB37347CAB42E2B1A45897C_old.css
> > > jquery-ver-F334A4E46CB37347CAB42E2B1A45897C_bakup.css
> > > jquery-ver-XYZABCDEF.css
> > > etc.
> > >
> > > Maybe we should add a check if version passed in request is correct?
> > > There is partially such check done in decorateResponse() method. So
> > > maybe it is enough to add else block here and raise some exception?
> > >
> > > @Override
> > > public void decorateResponse(AbstractResource.ResourceResponse
> > > response, IStaticCacheableResource resource) {
> > >   String requestedVersion =
> > > RequestCycle.get().getMetaData(URL_VERSION); String
> > > calculatedVersion = this.resourceVersion.getVersion(resource); if
> > > (calculatedVersion != null &&
> > > calculatedVersion.equals(requestedVersion))
> > > { response.setCacheDurationToMaximum();
> > > response.setCacheScope(WebResponse.CacheScope.PUBLIC); } }
> > >
> > > --
> > > Best regards,
> > > Daniel Stoch
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> > > For additional commands, e-mail: users-help@wicket.apache.org
> > >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> > For additional commands, e-mail: users-help@wicket.apache.org
> >
>
> 000
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org


Re: FilenameWithVersionResourceCachingStrategy accepts (almost) any string as a version

Posted by Carl-Eric Menzel <cm...@wicketbuch.de>.
Sorry, didn't mean to sound dismissive. It's a valid point, just I'm
not seeing that anybody could get to anything otherwise unavailable.

On Mon, 25 May 2020 21:02:08 +0200
Carl-Eric Menzel <cm...@wicketbuch.de> wrote:

> I think the point of this version decoration is not to ensure a
> particular version is requested, because typically only one version of
> a file is available in the application.
> 
> The point is instead to defeat any caching, both in the browser and by
> proxies, which might serve the user an outdated version. So I don't
> think there needs to be any checking of that string.
> 
> Or is there an actual security impact that I'm missing?
> 
> Carl-Eric
> 
> On Mon, 25 May 2020 20:47:36 +0200
> Daniel Stoch <da...@gmail.com> wrote:
> 
> > Hi,
> > 
> > Each resource in Wicket is decorated using a version string in a
> > file name by default. It is implemented in
> > FilenameWithVersionResourceCachingStrategy. Depending on DEVELOPMENT
> > or DEPLOYMENT mode it looks like:
> > jquery-ver-1590158412000.css
> > jquery-ver-F334A4E46CB37347CAB42E2B1A45897C.css
> > 
> > There is a small security issue, that this strategy does not check
> > if this version is correctly calculated for specific resource and
> > accepts any string as a version identifier, eg.:
> > jquery-ver-F334A4E46CB37347CAB42E2B1A45897C_old.css
> > jquery-ver-F334A4E46CB37347CAB42E2B1A45897C_bakup.css
> > jquery-ver-XYZABCDEF.css
> > etc.
> > 
> > Maybe we should add a check if version passed in request is correct?
> > There is partially such check done in decorateResponse() method. So
> > maybe it is enough to add else block here and raise some exception?
> > 
> > @Override
> > public void decorateResponse(AbstractResource.ResourceResponse
> > response, IStaticCacheableResource resource) {
> >   String requestedVersion =
> > RequestCycle.get().getMetaData(URL_VERSION); String
> > calculatedVersion = this.resourceVersion.getVersion(resource); if
> > (calculatedVersion != null &&
> > calculatedVersion.equals(requestedVersion))
> > { response.setCacheDurationToMaximum();
> > response.setCacheScope(WebResponse.CacheScope.PUBLIC); } }
> > 
> > --
> > Best regards,
> > Daniel Stoch
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> > For additional commands, e-mail: users-help@wicket.apache.org
> >   
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
> 

000

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org


Re: FilenameWithVersionResourceCachingStrategy accepts (almost) any string as a version

Posted by Carl-Eric Menzel <cm...@wicketbuch.de>.
I think the point of this version decoration is not to ensure a
particular version is requested, because typically only one version of
a file is available in the application.

The point is instead to defeat any caching, both in the browser and by
proxies, which might serve the user an outdated version. So I don't
think there needs to be any checking of that string.

Or is there an actual security impact that I'm missing?

Carl-Eric

On Mon, 25 May 2020 20:47:36 +0200
Daniel Stoch <da...@gmail.com> wrote:

> Hi,
> 
> Each resource in Wicket is decorated using a version string in a file
> name by default. It is implemented in
> FilenameWithVersionResourceCachingStrategy. Depending on DEVELOPMENT
> or DEPLOYMENT mode it looks like:
> jquery-ver-1590158412000.css
> jquery-ver-F334A4E46CB37347CAB42E2B1A45897C.css
> 
> There is a small security issue, that this strategy does not check if
> this version is correctly calculated for specific resource and accepts
> any string as a version identifier, eg.:
> jquery-ver-F334A4E46CB37347CAB42E2B1A45897C_old.css
> jquery-ver-F334A4E46CB37347CAB42E2B1A45897C_bakup.css
> jquery-ver-XYZABCDEF.css
> etc.
> 
> Maybe we should add a check if version passed in request is correct?
> There is partially such check done in decorateResponse() method. So
> maybe it is enough to add else block here and raise some exception?
> 
> @Override
> public void decorateResponse(AbstractResource.ResourceResponse
> response, IStaticCacheableResource resource) {
>   String requestedVersion =
> RequestCycle.get().getMetaData(URL_VERSION); String calculatedVersion
> = this.resourceVersion.getVersion(resource); if (calculatedVersion !=
> null && calculatedVersion.equals(requestedVersion))
> { response.setCacheDurationToMaximum();
> response.setCacheScope(WebResponse.CacheScope.PUBLIC); }
> }
> 
> --
> Best regards,
> Daniel Stoch
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org