You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shiro.apache.org by "Roberto C. Sánchez" <ro...@debian.org> on 2021/01/30 21:44:57 UTC

Re: Request for assistance to backport CVE-2020-13933 fix

Bump.

On Tue, Dec 22, 2020 at 09:30:47AM -0500, Roberto C. Sánchez wrote:
> On Mon, Dec 21, 2020 at 09:33:44PM +0100, Benjamin Marwell wrote:
> > Hi Roberto,
> > 
> > after talking to the PMC chair, I can give you three commit links.
> > 
> > https://github.com/apache/shiro/commit/042c59356cc6442345a9f935aed3e7603cb4dfad
> > https://github.com/apache/shiro/commit/5b1add9a4c4ed046b52cf2132ed0f264a22caf1d
> > https://github.com/apache/shiro/commit/1b9d8d99cd6d50d7114916508a13677a0fe6f345
> > 
> > I guess it is quite obvious what is inside these commits.
> > 
> Hi Ben,
> 
> This commits seem to have been made after the 1.6.0 release and before
> the 1.7.0 release.  My belief is that they address CVE-2020-17510.  Can
> you tell me if dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d is the commit
> that addresses CVE-2020-13933?  Are there other commits that go along
> with it to completely remedy CVE-2020-13933?
> 
> Regards,
> 
> -Roberto
> 
> -- 
> Roberto C. Sánchez

-- 
Roberto C. Sánchez

Re: Request for assistance to backport CVE-2020-13933 fix

Posted by "Roberto C. Sánchez" <ro...@debian.org>.
With attachments.

On Tue, May 04, 2021 at 09:15:50PM -0400, Roberto C. Sánchez wrote:
> I never received a reply to the below request.  Is it possible for
> someone to review the CVE-2020-13933 patch?
> 
> Additionally, I have prepared two patches for CVE-2020-17510, which I
> have attached to this email.  The adaptations were fairly minor, but a
> review would still be appreciated.
> 
> As far as CVE-2020-17523, it looks the affected file
> (support/spring-boot/spring-boot-web-starter/src/main/resources/META-INF/spring.factories)
> was introduced in 1.4.x.  If that is the case, then CVE-2020-17523 would
> not affect 1.3.x versions of shiro.  Do I have that right?
> 
> I'd like to get these changes wrapped up and a review would increase my
> confidence in the adaptations I have made.
> 
> Once these are nailed down (fixes for CVE-2020-13933 and CVE-2020-17510,
> and confirmation that CVE-2020-17523 is not applicable in 1.3.x), then I
> will be in a position to address getting a more recent upstream release
> into Debian.
> 
> Regards,
> 
> -Roberto
> 
> On Wed, Apr 07, 2021 at 03:41:25PM -0400, Roberto C. Sánchez wrote:
> > Hi Brian, Ben, et al.,
> > 
> > I have what I believe to be a correct backport of commit dc194fc977ab to
> > address CVE-2020-13933.  The adaptations/changes include:
> > 
> > - replacing the diamond operator <> with type specifications (the 1.3.x
> >   build uses source compatibility level 1.6)
> > - replacing stream constructs with loops
> > - omitting changes to non-existent tests
> > - adjusting for the absence of the FilterConfig type (which seems to
> >   have been introduced in 1.4)
> > 
> > If someone familiar with the code, the vulnerability, and the fix could
> > review the attached patch that would be very much appreciated.  Also, it
> > would be quite helpful if there was a reproducer or a description of how
> > to exploit the vulnerability to help with validating the fix.  If that
> > cannot be disclosed, then perhaps someone could confirm that this patch
> > does in fact address the vulnerability; that would be helpful.
> > 
> > Next I will start working on CVE-2020-17510 and CVE-2020-17523.  Once I
> > have those sorted out for the existing 1.3.x packages I will turn my
> > attention to seeing about updating to a newer upstream release (which
> > could be a lengthy process).
> > 
> > Regards,
> > 
> > -Roberto
> > 
> > On Wed, Mar 31, 2021 at 03:21:55PM -0400, Roberto C. Sánchez wrote:
> > > Hi Brian,
> > > 
> > > Thanks for your help.  I am working to backport dc194fc977ab to address
> > > CVE-2020-13933 and after that I will move on to the fixes for
> > > CVE-2020-17510 and CVE-2020-17523.
> > > 
> > > As far as the maintainability of a 1.3.x package, upgrading to a newer
> > > version is not an option for two reasons.  First, I am working on
> > > addressing these vulnerabilities in Debian Stretch, which is in the LTS
> > > stage of its lifecycle, making an update to a new upstream release very
> > > unlikely.  Second, the shiro package in Debian was last updated about 2
> > > years ago and even in Debian unstable 1.3.x is the highest available
> > > version.  Any update to a new upstream version would need to start in
> > > the unstable distribution.
> > > 
> > > As an additional complication, the activemq package in Debian depends on
> > > the shiro package, so a shiro update would need to be coordinated in a
> > > that ensures it doesn't break activemq.
> > > 
> > > That said, I will bring the issue up to see if I can get an update
> > > started so that at the least the next Debian stable release has
> > > something more recent.
> > > 
> > > Regards,
> > > 
> > > -Roberto
> > > 
> > > On Tue, Mar 16, 2021 at 05:50:50PM -0400, Brian Demers wrote:
> > > > Hey Roberto,
> > > > 
> > > > Sorry about the delay on this one, I originally thought we had answered
> > > > your question.
> > > > 
> > > > The commit you are looking for is
> > > > https://github.com/apache/shiro/commit/dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d
> > > > 
> > > > If you are maintaining a 1.3.x package this is going to become more
> > > > difficult, is it possible to deprecate it and move to a recent version?
> > > > 
> > > > 
> > > > 
> > > > On Sat, Jan 30, 2021 at 4:45 PM Roberto C. Sánchez <ro...@debian.org>
> > > > wrote:
> > > > 
> > > > > Bump.
> > > > >
> > > > > On Tue, Dec 22, 2020 at 09:30:47AM -0500, Roberto C. Sánchez wrote:
> > > > > > On Mon, Dec 21, 2020 at 09:33:44PM +0100, Benjamin Marwell wrote:
> > > > > > > Hi Roberto,
> > > > > > >
> > > > > > > after talking to the PMC chair, I can give you three commit links.
> > > > > > >
> > > > > > >
> > > > > https://github.com/apache/shiro/commit/042c59356cc6442345a9f935aed3e7603cb4dfad
> > > > > > >
> > > > > https://github.com/apache/shiro/commit/5b1add9a4c4ed046b52cf2132ed0f264a22caf1d
> > > > > > >
> > > > > https://github.com/apache/shiro/commit/1b9d8d99cd6d50d7114916508a13677a0fe6f345
> > > > > > >
> > > > > > > I guess it is quite obvious what is inside these commits.
> > > > > > >
> > > > > > Hi Ben,
> > > > > >
> > > > > > This commits seem to have been made after the 1.6.0 release and before
> > > > > > the 1.7.0 release.  My belief is that they address CVE-2020-17510.  Can
> > > > > > you tell me if dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d is the commit
> > > > > > that addresses CVE-2020-13933?  Are there other commits that go along
> > > > > > with it to completely remedy CVE-2020-13933?
> > > > > >
> > > > > > Regards,
> > > > > >
> > > > > > -Roberto
> > > > > >
> > > > > > --
> > > > > > Roberto C. Sánchez
> > > > >
> > > > > --
> > > > > Roberto C. Sánchez
> > > > >
> > > 
> > > -- 
> > > Roberto C. Sánchez
> > > http://people.connexer.com/~roberto
> > > http://www.connexer.com
> > 
> > -- 
> > Roberto C. Sánchez
> > http://people.connexer.com/~roberto
> > http://www.connexer.com
> 
> > From dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d Mon Sep 17 00:00:00 2001
> > From: Brian Demers <bd...@apache.org>
> > Date: Tue, 7 Jul 2020 21:06:35 -0400
> > Subject: [PATCH] Add a feature to allow for global filters
> > 
> > Adds new filter to block invalid requests
> > ---
> >  .../shiro/guice/web/ShiroWebModule.java       |  25 ++-
> >  .../shiro/guice/web/ShiroWebModuleTest.java   | 153 ++++++++++++++++++
> >  .../ShiroWebFilterConfiguration.java          |   8 +
> >  .../web/ConfiguredGlobalFiltersTest.groovy    | 104 ++++++++++++
> >  .../web/DisabledGlobalFiltersTest.groovy      |  64 ++++++++
> >  ...ShiroWebSpringAutoConfigurationTest.groovy |  30 +++-
> >  ...roWebAutoConfigurationTestApplication.java |   4 +-
> >  .../spring/web/ShiroFilterFactoryBean.java    |  23 +++
> >  .../config/AbstractShiroWebConfiguration.java |   3 -
> >  .../AbstractShiroWebFilterConfiguration.java  |   9 +-
> >  .../config/ShiroWebFilterConfiguration.java   |   6 +
> >  .../ShiroWebFilterConfigurationTest.groovy    |   3 +-
> >  .../web/ShiroFilterFactoryBeanTest.java       |   8 +-
> >  .../config/IniFilterChainResolverFactory.java |  18 +++
> >  .../web/filter/InvalidRequestFilter.java      | 124 ++++++++++++++
> >  .../shiro/web/filter/mgt/DefaultFilter.java   |   4 +-
> >  .../filter/mgt/DefaultFilterChainManager.java |  37 ++++-
> >  .../web/filter/mgt/FilterChainManager.java    |  22 +++
> >  .../web/servlet/AbstractShiroFilter.java      |   1 +
> >  .../IniFilterChainResolverFactoryTest.groovy  |  26 +++
> >  .../web/env/IniWebEnvironmentTest.groovy      |  69 ++++++++
> >  .../filter/InvalidRequestFilterTest.groovy    | 106 ++++++++++++
> >  .../mgt/DefaultFilterChainManagerTest.groovy  |  52 ++++++
> >  .../org/apache/shiro/web/env/FilterStub.java  |  45 ++++++
> >  24 files changed, 925 insertions(+), 19 deletions(-)
> >  create mode 100644 support/spring-boot/spring-boot-web-starter/src/test/groovy/org/apache/shiro/spring/boot/autoconfigure/web/ConfiguredGlobalFiltersTest.groovy
> >  create mode 100644 support/spring-boot/spring-boot-web-starter/src/test/groovy/org/apache/shiro/spring/boot/autoconfigure/web/DisabledGlobalFiltersTest.groovy
> >  create mode 100644 web/src/main/java/org/apache/shiro/web/filter/InvalidRequestFilter.java
> >  create mode 100644 web/src/test/groovy/org/apache/shiro/web/filter/InvalidRequestFilterTest.groovy
> >  create mode 100644 web/src/test/java/org/apache/shiro/web/env/FilterStub.java
> > 
> > --- a/support/guice/src/main/java/org/apache/shiro/guice/web/ShiroWebModule.java
> > +++ b/support/guice/src/main/java/org/apache/shiro/guice/web/ShiroWebModule.java
> > @@ -18,9 +18,13 @@
> >   */
> >  package org.apache.shiro.guice.web;
> >  
> > +import java.util.ArrayList;
> > +import java.util.Arrays;
> >  import java.util.Collection;
> > +import java.util.Collections;
> >  import java.util.HashMap;
> >  import java.util.LinkedHashMap;
> > +import java.util.List;
> >  import java.util.Map;
> >  
> >  import javax.servlet.Filter;
> > @@ -32,6 +36,7 @@
> >  import org.apache.shiro.mgt.SecurityManager;
> >  import org.apache.shiro.session.mgt.SessionManager;
> >  import org.apache.shiro.web.env.WebEnvironment;
> > +import org.apache.shiro.web.filter.InvalidRequestFilter;;
> >  import org.apache.shiro.web.filter.PathMatchingFilter;
> >  import org.apache.shiro.web.filter.authc.AnonymousFilter;
> >  import org.apache.shiro.web.filter.authc.BasicHttpAuthenticationFilter;
> > @@ -86,7 +91,8 @@
> >      public static final Key<SslFilter> SSL = Key.get(SslFilter.class);
> >      @SuppressWarnings({"UnusedDeclaration"})
> >      public static final Key<UserFilter> USER = Key.get(UserFilter.class);
> > -
> > +    @SuppressWarnings({"UnusedDeclaration"})
> > +    public static final Key<InvalidRequestFilter> INVALID_REQUEST = Key.get(InvalidRequestFilter.class);
> >  
> >      static final String NAME = "SHIRO";
> >  
> > @@ -123,6 +129,12 @@
> >          };
> >      }
> >  
> > +    public List<Key<? extends Filter>> globalFilters() {
> > +        ArrayList<Key<? extends Filter>> filters = new ArrayList<Key<? extends Filter>>();
> > +        filters.add(INVALID_REQUEST);
> > +        return Collections.unmodifiableList(filters);
> > +    }
> > +
> >      @Override
> >      protected final void configureShiro() {
> >          bindBeanType(TypeLiteral.get(ServletContext.class), Key.get(ServletContext.class, Names.named(NAME)));
> > @@ -134,6 +146,12 @@
> >  
> >          this.configureShiroWeb();
> >  
> > +        // add default matching route if not already set
> > +        if (!filterChains.containsKey("/**")) {
> > +            // no config, this will add only the global filters
> > +            this.addFilterChain("/**", new Key[0]);
> > +        }
> > +
> >          setupFilterChainConfigs();
> >  
> >          bind(FilterChainResolver.class).toProvider(new FilterChainResolverProvider(filterChains));
> > @@ -143,8 +161,15 @@
> >          Map<Key<? extends PathMatchingFilter>, Map<String, String>> configs = new HashMap<Key<? extends PathMatchingFilter>, Map<String, String>>();
> >  
> >          for (Map.Entry<String, Key<? extends Filter>[]> filterChain : filterChains.entrySet()) {
> > -            for (int i = 0; i < filterChain.getValue().length; i++) {
> > -                Key<? extends Filter> key = filterChain.getValue()[i];
> > +            List<Key<? extends Filter>> globalFilters = this.globalFilters();
> > +            Key<? extends Filter>[] pathFilters = filterChain.getValue();
> > +
> > +            // merge the global filters and the path specific filters
> > +            List<Key<? extends Filter>> filterConfigs = new ArrayList<Key<? extends Filter>>(globalFilters.size() + pathFilters.length);
> > +            filterConfigs.addAll(globalFilters);
> > +            filterConfigs.addAll(Arrays.asList(pathFilters));
> > +            for (int i = 0; i < filterConfigs.size(); i++) {
> > +                Key<? extends Filter> key = filterConfigs.get(i);
> >                  if (key instanceof FilterConfigKey) {
> >                      FilterConfigKey<? extends PathMatchingFilter> configKey = (FilterConfigKey<? extends PathMatchingFilter>) key;
> >                      key = configKey.getKey();
> > --- a/support/spring/src/main/java/org/apache/shiro/spring/web/ShiroFilterFactoryBean.java
> > +++ b/support/spring/src/main/java/org/apache/shiro/spring/web/ShiroFilterFactoryBean.java
> > @@ -25,8 +25,10 @@
> >  import org.apache.shiro.util.StringUtils;
> >  import org.apache.shiro.web.config.IniFilterChainResolverFactory;
> >  import org.apache.shiro.web.filter.AccessControlFilter;
> > +import org.apache.shiro.web.filter.InvalidRequestFilter;
> >  import org.apache.shiro.web.filter.authc.AuthenticationFilter;
> >  import org.apache.shiro.web.filter.authz.AuthorizationFilter;
> > +import org.apache.shiro.web.filter.mgt.DefaultFilter;
> >  import org.apache.shiro.web.filter.mgt.DefaultFilterChainManager;
> >  import org.apache.shiro.web.filter.mgt.FilterChainManager;
> >  import org.apache.shiro.web.filter.mgt.FilterChainResolver;
> > @@ -41,7 +43,9 @@
> >  import org.springframework.beans.factory.config.BeanPostProcessor;
> >  
> >  import javax.servlet.Filter;
> > +import java.util.ArrayList;
> >  import java.util.LinkedHashMap;
> > +import java.util.List;
> >  import java.util.Map;
> >  
> >  /**
> > @@ -121,6 +125,8 @@
> >  
> >      private Map<String, Filter> filters;
> >  
> > +    private List<String> globalFilters;
> > +
> >      private Map<String, String> filterChainDefinitionMap; //urlPathExpression_to_comma-delimited-filter-chain-definition
> >  
> >      private String loginUrl;
> > @@ -131,6 +137,8 @@
> >  
> >      public ShiroFilterFactoryBean() {
> >          this.filters = new LinkedHashMap<String, Filter>();
> > +        this.globalFilters = new ArrayList<String>();
> > +        this.globalFilters.add(DefaultFilter.invalidRequest.name());
> >          this.filterChainDefinitionMap = new LinkedHashMap<String, String>(); //order matters!
> >      }
> >  
> > @@ -332,6 +340,14 @@
> >      }
> >  
> >      /**
> > +     * Sets the list of filters that will be executed against every request.  Defaults to the {@link InvalidRequestFilter} which will block known invalid request attacks.
> > +     * @param globalFilters the list of filters to execute before specific path filters.
> > +     */
> > +    public void setGlobalFilters(List<String> globalFilters) {
> > +        this.globalFilters = globalFilters;
> > +    }
> > +
> > +    /**
> >       * Lazily creates and returns a {@link AbstractShiroFilter} concrete instance via the
> >       * {@link #createInstance} method.
> >       *
> > @@ -388,6 +404,9 @@
> >              }
> >          }
> >  
> > +        // set the global filters
> > +        manager.setGlobalFilters(this.globalFilters);
> > +
> >          //build up the chains:
> >          Map<String, String> chains = getFilterChainDefinitionMap();
> >          if (!CollectionUtils.isEmpty(chains)) {
> > @@ -398,6 +417,9 @@
> >              }
> >          }
> >  
> > +        // create the default chain, to match anything the path matching would have missed
> > +        manager.createDefaultChain("/**"); // TODO this assumes ANT path matching, which might be OK here
> > +
> >          return manager;
> >      }
> >  
> > --- a/web/src/main/java/org/apache/shiro/web/config/IniFilterChainResolverFactory.java
> > +++ b/web/src/main/java/org/apache/shiro/web/config/IniFilterChainResolverFactory.java
> > @@ -24,6 +24,7 @@
> >  import org.apache.shiro.config.ReflectionBuilder;
> >  import org.apache.shiro.util.CollectionUtils;
> >  import org.apache.shiro.util.Factory;
> > +import org.apache.shiro.web.filter.mgt.DefaultFilter;
> >  import org.apache.shiro.web.filter.mgt.FilterChainManager;
> >  import org.apache.shiro.web.filter.mgt.FilterChainResolver;
> >  import org.apache.shiro.web.filter.mgt.PathMatchingFilterChainResolver;
> > @@ -32,7 +33,9 @@
> >  
> >  import javax.servlet.Filter;
> >  import javax.servlet.FilterConfig;
> > +import java.util.Collections;
> >  import java.util.LinkedHashMap;
> > +import java.util.List;
> >  import java.util.Map;
> >  
> >  /**
> > @@ -49,6 +52,8 @@
> >  
> >      private FilterConfig filterConfig;
> >  
> > +    private List<String> globalFilters = Collections.singletonList(DefaultFilter.invalidRequest.name());
> > +
> >      private Map<String, ?> defaultBeans;
> >  
> >      public IniFilterChainResolverFactory() {
> > @@ -72,6 +77,14 @@
> >          this.filterConfig = filterConfig;
> >      }
> >  
> > +    public List<String> getGlobalFilters() {
> > +        return globalFilters;
> > +    }
> > +
> > +    public void setGlobalFilters(List<String> globalFilters) {
> > +        this.globalFilters = globalFilters;
> > +    }
> > +
> >      protected FilterChainResolver createInstance(Ini ini) {
> >          FilterChainResolver filterChainResolver = createDefaultInstance();
> >          if (filterChainResolver instanceof PathMatchingFilterChainResolver) {
> > @@ -122,9 +135,14 @@
> >          //add the filters to the manager:
> >          registerFilters(filters, manager);
> >  
> > +        manager.setGlobalFilters(getGlobalFilters());
> > +
> >          //urls section:
> >          section = ini.getSection(URLS);
> >          createChains(section, manager);
> > +
> > +        // create the default chain, to match anything the path matching would have missed
> > +        manager.createDefaultChain("/**"); // TODO this assumes ANT path matching
> >      }
> >  
> >      protected void registerFilters(Map<String, Filter> filters, FilterChainManager manager) {
> > --- /dev/null
> > +++ b/web/src/main/java/org/apache/shiro/web/filter/InvalidRequestFilter.java
> > @@ -0,0 +1,136 @@
> > +/*
> > + * Licensed to the Apache Software Foundation (ASF) under one
> > + * or more contributor license agreements.  See the NOTICE file
> > + * distributed with this work for additional information
> > + * regarding copyright ownership.  The ASF licenses this file
> > + * to you under the Apache License, Version 2.0 (the
> > + * "License"); you may not use this file except in compliance
> > + * with the License.  You may obtain a copy of the License at
> > + *
> > + *     http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing,
> > + * software distributed under the License is distributed on an
> > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> > + * KIND, either express or implied.  See the License for the
> > + * specific language governing permissions and limitations
> > + * under the License.
> > + */
> > +
> > +package org.apache.shiro.web.filter;
> > +
> > +import org.apache.shiro.web.util.WebUtils;
> > +
> > +import javax.servlet.ServletRequest;
> > +import javax.servlet.ServletResponse;
> > +import java.util.Arrays;
> > +import java.util.Collections;
> > +import java.util.List;
> > +
> > +/**
> > + * A request filter that blocks malicious requests. Invalid request will respond with a 400 response code.
> > + * <p>
> > + * This filter checks and blocks the request if the following characters are found in the request URI:
> > + * <ul>
> > + *     <li>Semicolon - can be disabled by setting {@code blockSemicolon = false}</li>
> > + *     <li>Backslash - can be disabled by setting {@code blockBackslash = false}</li>
> > + *     <li>Non-ASCII characters - can be disabled by setting {@code blockNonAscii = false}, the ability to disable this check will be removed in future version.</li>
> > + * </ul>
> > + *
> > + * @see <a href="https://docs.spring.io/spring-security/site/docs/current/api/org/springframework/security/web/firewall/StrictHttpFirewall.html">This class was inspired by Spring Security StrictHttpFirewall</a>
> > + * @since 1.6
> > + */
> > +public class InvalidRequestFilter extends AccessControlFilter {
> > +
> > +    private static final List<String> SEMICOLON = Collections.unmodifiableList(Arrays.asList(";", "%3b", "%3B"));
> > +
> > +    private static final List<String> BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C"));
> > +
> > +    private boolean blockSemicolon = true;
> > +
> > +    private boolean blockBackslash = true;
> > +
> > +    private boolean blockNonAscii = true;
> > +
> > +    @Override
> > +    protected boolean isAccessAllowed(ServletRequest request, ServletResponse response, Object mappedValue) throws Exception {
> > +        String uri = WebUtils.toHttp(request).getRequestURI();
> > +        return !containsSemicolon(uri)
> > +            && !containsBackslash(uri)
> > +            && !containsNonAsciiCharacters(uri);
> > +    }
> > +
> > +    @Override
> > +    protected boolean onAccessDenied(ServletRequest request, ServletResponse response) throws Exception {
> > +        WebUtils.toHttp(response).sendError(400, "Invalid request");
> > +        return false;
> > +    }
> > +
> > +    private boolean containsSemicolon(String uri) {
> > +        if (isBlockSemicolon()) {
> > +            int length = uri.length();
> > +            for (int i = 0; i < length; i++) {
> > +                char c = uri.charAt(i);
> > +                if (c == ';') {
> > +                    return true;
> > +                }
> > +            }
> > +        }
> > +        return false;
> > +    }
> > +
> > +    private boolean containsBackslash(String uri) {
> > +        if (isBlockBackslash()) {
> > +            int length = uri.length();
> > +            for (int i = 0; i < length; i++) {
> > +                char c = uri.charAt(i);
> > +                if (c == '\\') {
> > +                    return true;
> > +                }
> > +            }
> > +        }
> > +        return false;
> > +    }
> > +
> > +    private boolean containsNonAsciiCharacters(String uri) {
> > +        if (isBlockNonAscii()) {
> > +            return !containsOnlyPrintableAsciiCharacters(uri);
> > +        }
> > +        return false;
> > +    }
> > +
> > +    private static boolean containsOnlyPrintableAsciiCharacters(String uri) {
> > +        int length = uri.length();
> > +        for (int i = 0; i < length; i++) {
> > +            char c = uri.charAt(i);
> > +            if (c < '\u0020' || c > '\u007e') {
> > +                return false;
> > +            }
> > +        }
> > +        return true;
> > +    }
> > +
> > +    public boolean isBlockSemicolon() {
> > +        return blockSemicolon;
> > +    }
> > +
> > +    public void setBlockSemicolon(boolean blockSemicolon) {
> > +        this.blockSemicolon = blockSemicolon;
> > +    }
> > +
> > +    public boolean isBlockBackslash() {
> > +        return blockBackslash;
> > +    }
> > +
> > +    public void setBlockBackslash(boolean blockBackslash) {
> > +        this.blockBackslash = blockBackslash;
> > +    }
> > +
> > +    public boolean isBlockNonAscii() {
> > +        return blockNonAscii;
> > +    }
> > +
> > +    public void setBlockNonAscii(boolean blockNonAscii) {
> > +        this.blockNonAscii = blockNonAscii;
> > +    }
> > +}
> > --- a/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilter.java
> > +++ b/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilter.java
> > @@ -19,6 +19,7 @@
> >  package org.apache.shiro.web.filter.mgt;
> >  
> >  import org.apache.shiro.util.ClassUtils;
> > +import org.apache.shiro.web.filter.InvalidRequestFilter;
> >  import org.apache.shiro.web.filter.authc.*;
> >  import org.apache.shiro.web.filter.authz.*;
> >  import org.apache.shiro.web.filter.session.NoSessionCreationFilter;
> > @@ -47,7 +48,8 @@
> >      rest(HttpMethodPermissionFilter.class),
> >      roles(RolesAuthorizationFilter.class),
> >      ssl(SslFilter.class),
> > -    user(UserFilter.class);
> > +    user(UserFilter.class),
> > +    invalidRequest(InvalidRequestFilter.class);
> >  
> >      private final Class<? extends Filter> filterClass;
> >  
> > --- a/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilterChainManager.java
> > +++ b/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilterChainManager.java
> > @@ -30,8 +30,10 @@
> >  import javax.servlet.FilterChain;
> >  import javax.servlet.FilterConfig;
> >  import javax.servlet.ServletException;
> > +import java.util.ArrayList;
> >  import java.util.Collections;
> >  import java.util.LinkedHashMap;
> > +import java.util.List;
> >  import java.util.Map;
> >  import java.util.Set;
> >  
> > @@ -52,17 +54,21 @@
> >  
> >      private Map<String, Filter> filters; //pool of filters available for creating chains
> >  
> > +    private List<String> globalFilterNames; // list of filters to prepend to every chain
> > +
> >      private Map<String, NamedFilterList> filterChains; //key: chain name, value: chain
> >  
> >      public DefaultFilterChainManager() {
> >          this.filters = new LinkedHashMap<String, Filter>();
> >          this.filterChains = new LinkedHashMap<String, NamedFilterList>();
> > +        this.globalFilterNames = new ArrayList<String>();
> >          addDefaultFilters(false);
> >      }
> >  
> >      public DefaultFilterChainManager(FilterConfig filterConfig) {
> >          this.filters = new LinkedHashMap<String, Filter>();
> >          this.filterChains = new LinkedHashMap<String, NamedFilterList>();
> > +        this.globalFilterNames = new ArrayList<String>();
> >          setFilterConfig(filterConfig);
> >          addDefaultFilters(true);
> >      }
> > @@ -115,6 +121,17 @@
> >          addFilter(name, filter, init, true);
> >      }
> >  
> > +    public void createDefaultChain(String chainName) {
> > +        // only create the defaultChain if we don't have a chain with this name already
> > +        // (the global filters will already be in that chain)
> > +        if (!getChainNames().contains(chainName) && !CollectionUtils.isEmpty(globalFilterNames)) {
> > +            // add each of global filters
> > +            for (String filterName : globalFilterNames) {
> > +                addToChain(chainName, filterName);
> > +            }
> > +        }
> > +    }
> > +
> >      public void createChain(String chainName, String chainDefinition) {
> >          if (!StringUtils.hasText(chainName)) {
> >              throw new NullPointerException("chainName cannot be null or empty.");
> > @@ -124,7 +141,14 @@
> >          }
> >  
> >          if (log.isDebugEnabled()) {
> > -            log.debug("Creating chain [" + chainName + "] from String definition [" + chainDefinition + "]");
> > +            log.debug("Creating chain [" + chainName + "] with global filters " + globalFilterNames + " and from String definition [" + chainDefinition + "]");
> > +        }
> > +
> > +        // first add each of global filters
> > +        if (!CollectionUtils.isEmpty(globalFilterNames)) {
> > +            for (String filterName : globalFilterNames) {
> > +                addToChain(chainName, filterName);
> > +            }
> >          }
> >  
> >          //parse the value by tokenizing it to get the resulting filter-specific config entries
> > @@ -273,6 +297,21 @@
> >          chain.add(filter);
> >      }
> >  
> > +    public void setGlobalFilters(List<String> globalFilterNames) throws ConfigurationException {
> > +        // validate each filter name
> > +        if (!CollectionUtils.isEmpty(globalFilterNames)) {
> > +            for (String filterName : globalFilterNames) {
> > +                Filter filter = filters.get(filterName);
> > +                if (filter == null) {
> > +                    throw new ConfigurationException("There is no filter with name '" + filterName +
> > +                                                     "' to apply to the global filters in the pool of available Filters.  Ensure a " +
> > +                                                     "filter with that name/path has first been registered with the addFilter method(s).");
> > +                }
> > +                this.globalFilterNames.add(filterName);
> > +            }
> > +        }
> > +    }
> > +
> >      protected void applyChainConfig(String chainName, Filter filter, String chainSpecificFilterConfig) {
> >          if (log.isDebugEnabled()) {
> >              log.debug("Attempting to apply path [" + chainName + "] to filter [" + filter + "] " +
> > --- a/web/src/main/java/org/apache/shiro/web/filter/mgt/FilterChainManager.java
> > +++ b/web/src/main/java/org/apache/shiro/web/filter/mgt/FilterChainManager.java
> > @@ -22,6 +22,7 @@
> >  
> >  import javax.servlet.Filter;
> >  import javax.servlet.FilterChain;
> > +import java.util.List;
> >  import java.util.Map;
> >  import java.util.Set;
> >  
> > @@ -165,6 +166,14 @@
> >      void createChain(String chainName, String chainDefinition);
> >  
> >      /**
> > +     * Creates a chain that should match any non-matched request paths, typically {@code /**} assuming an {@link AntPathMatcher} I used.
> > +     * @param chainName The name of the chain to create, likely {@code /**}.
> > +     * @since 1.6
> > +     * @see org.apache.shiro.lang.util.AntPathMatcher AntPathMatcher
> > +     */
> > +    void createDefaultChain(String chainName);
> > +
> > +    /**
> >       * Adds (appends) a filter to the filter chain identified by the given {@code chainName}.  If there is no chain
> >       * with the given name, a new one is created and the filter will be the first in the chain.
> >       *
> > @@ -195,4 +204,17 @@
> >       *                                  interface).
> >       */
> >      void addToChain(String chainName, String filterName, String chainSpecificFilterConfig) throws ConfigurationException;
> > +
> > +    /**
> > +     * Configures the set of named filters that will match all paths.  These filters will match BEFORE explicitly
> > +     * configured filter chains i.e. by calling {@link #createChain(String, String)}, {@link #addToChain(String, String)}, etc.
> > +     * <br>
> > +     * <strong>Filters configured in this list wll apply to ALL requests.</strong>
> > +     *
> > +     * @param globalFilterNames         the list of filter names to match ALL paths.
> > +     * @throws ConfigurationException   if one of the filter names is invalid and cannot be loaded from the set of
> > +     *                                  configured filters {@link #getFilters()}}.
> > +     * @since 1.6
> > +     */
> > +    void setGlobalFilters(List<String> globalFilterNames) throws ConfigurationException;
> >  }
> 
> 
> -- 
> Roberto C. Sánchez

-- 
Roberto C. Sánchez

Re: Request for assistance to backport CVE-2020-13933 fix

Posted by "Roberto C. Sánchez" <ro...@debian.org>.
I never received a reply to the below request.  Is it possible for
someone to review the CVE-2020-13933 patch?

Additionally, I have prepared two patches for CVE-2020-17510, which I
have attached to this email.  The adaptations were fairly minor, but a
review would still be appreciated.

As far as CVE-2020-17523, it looks the affected file
(support/spring-boot/spring-boot-web-starter/src/main/resources/META-INF/spring.factories)
was introduced in 1.4.x.  If that is the case, then CVE-2020-17523 would
not affect 1.3.x versions of shiro.  Do I have that right?

I'd like to get these changes wrapped up and a review would increase my
confidence in the adaptations I have made.

Once these are nailed down (fixes for CVE-2020-13933 and CVE-2020-17510,
and confirmation that CVE-2020-17523 is not applicable in 1.3.x), then I
will be in a position to address getting a more recent upstream release
into Debian.

Regards,

-Roberto

On Wed, Apr 07, 2021 at 03:41:25PM -0400, Roberto C. Sánchez wrote:
> Hi Brian, Ben, et al.,
> 
> I have what I believe to be a correct backport of commit dc194fc977ab to
> address CVE-2020-13933.  The adaptations/changes include:
> 
> - replacing the diamond operator <> with type specifications (the 1.3.x
>   build uses source compatibility level 1.6)
> - replacing stream constructs with loops
> - omitting changes to non-existent tests
> - adjusting for the absence of the FilterConfig type (which seems to
>   have been introduced in 1.4)
> 
> If someone familiar with the code, the vulnerability, and the fix could
> review the attached patch that would be very much appreciated.  Also, it
> would be quite helpful if there was a reproducer or a description of how
> to exploit the vulnerability to help with validating the fix.  If that
> cannot be disclosed, then perhaps someone could confirm that this patch
> does in fact address the vulnerability; that would be helpful.
> 
> Next I will start working on CVE-2020-17510 and CVE-2020-17523.  Once I
> have those sorted out for the existing 1.3.x packages I will turn my
> attention to seeing about updating to a newer upstream release (which
> could be a lengthy process).
> 
> Regards,
> 
> -Roberto
> 
> On Wed, Mar 31, 2021 at 03:21:55PM -0400, Roberto C. Sánchez wrote:
> > Hi Brian,
> > 
> > Thanks for your help.  I am working to backport dc194fc977ab to address
> > CVE-2020-13933 and after that I will move on to the fixes for
> > CVE-2020-17510 and CVE-2020-17523.
> > 
> > As far as the maintainability of a 1.3.x package, upgrading to a newer
> > version is not an option for two reasons.  First, I am working on
> > addressing these vulnerabilities in Debian Stretch, which is in the LTS
> > stage of its lifecycle, making an update to a new upstream release very
> > unlikely.  Second, the shiro package in Debian was last updated about 2
> > years ago and even in Debian unstable 1.3.x is the highest available
> > version.  Any update to a new upstream version would need to start in
> > the unstable distribution.
> > 
> > As an additional complication, the activemq package in Debian depends on
> > the shiro package, so a shiro update would need to be coordinated in a
> > that ensures it doesn't break activemq.
> > 
> > That said, I will bring the issue up to see if I can get an update
> > started so that at the least the next Debian stable release has
> > something more recent.
> > 
> > Regards,
> > 
> > -Roberto
> > 
> > On Tue, Mar 16, 2021 at 05:50:50PM -0400, Brian Demers wrote:
> > > Hey Roberto,
> > > 
> > > Sorry about the delay on this one, I originally thought we had answered
> > > your question.
> > > 
> > > The commit you are looking for is
> > > https://github.com/apache/shiro/commit/dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d
> > > 
> > > If you are maintaining a 1.3.x package this is going to become more
> > > difficult, is it possible to deprecate it and move to a recent version?
> > > 
> > > 
> > > 
> > > On Sat, Jan 30, 2021 at 4:45 PM Roberto C. Sánchez <ro...@debian.org>
> > > wrote:
> > > 
> > > > Bump.
> > > >
> > > > On Tue, Dec 22, 2020 at 09:30:47AM -0500, Roberto C. Sánchez wrote:
> > > > > On Mon, Dec 21, 2020 at 09:33:44PM +0100, Benjamin Marwell wrote:
> > > > > > Hi Roberto,
> > > > > >
> > > > > > after talking to the PMC chair, I can give you three commit links.
> > > > > >
> > > > > >
> > > > https://github.com/apache/shiro/commit/042c59356cc6442345a9f935aed3e7603cb4dfad
> > > > > >
> > > > https://github.com/apache/shiro/commit/5b1add9a4c4ed046b52cf2132ed0f264a22caf1d
> > > > > >
> > > > https://github.com/apache/shiro/commit/1b9d8d99cd6d50d7114916508a13677a0fe6f345
> > > > > >
> > > > > > I guess it is quite obvious what is inside these commits.
> > > > > >
> > > > > Hi Ben,
> > > > >
> > > > > This commits seem to have been made after the 1.6.0 release and before
> > > > > the 1.7.0 release.  My belief is that they address CVE-2020-17510.  Can
> > > > > you tell me if dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d is the commit
> > > > > that addresses CVE-2020-13933?  Are there other commits that go along
> > > > > with it to completely remedy CVE-2020-13933?
> > > > >
> > > > > Regards,
> > > > >
> > > > > -Roberto
> > > > >
> > > > > --
> > > > > Roberto C. Sánchez
> > > >
> > > > --
> > > > Roberto C. Sánchez
> > > >
> > 
> > -- 
> > Roberto C. Sánchez
> > http://people.connexer.com/~roberto
> > http://www.connexer.com
> 
> -- 
> Roberto C. Sánchez
> http://people.connexer.com/~roberto
> http://www.connexer.com

> From dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d Mon Sep 17 00:00:00 2001
> From: Brian Demers <bd...@apache.org>
> Date: Tue, 7 Jul 2020 21:06:35 -0400
> Subject: [PATCH] Add a feature to allow for global filters
> 
> Adds new filter to block invalid requests
> ---
>  .../shiro/guice/web/ShiroWebModule.java       |  25 ++-
>  .../shiro/guice/web/ShiroWebModuleTest.java   | 153 ++++++++++++++++++
>  .../ShiroWebFilterConfiguration.java          |   8 +
>  .../web/ConfiguredGlobalFiltersTest.groovy    | 104 ++++++++++++
>  .../web/DisabledGlobalFiltersTest.groovy      |  64 ++++++++
>  ...ShiroWebSpringAutoConfigurationTest.groovy |  30 +++-
>  ...roWebAutoConfigurationTestApplication.java |   4 +-
>  .../spring/web/ShiroFilterFactoryBean.java    |  23 +++
>  .../config/AbstractShiroWebConfiguration.java |   3 -
>  .../AbstractShiroWebFilterConfiguration.java  |   9 +-
>  .../config/ShiroWebFilterConfiguration.java   |   6 +
>  .../ShiroWebFilterConfigurationTest.groovy    |   3 +-
>  .../web/ShiroFilterFactoryBeanTest.java       |   8 +-
>  .../config/IniFilterChainResolverFactory.java |  18 +++
>  .../web/filter/InvalidRequestFilter.java      | 124 ++++++++++++++
>  .../shiro/web/filter/mgt/DefaultFilter.java   |   4 +-
>  .../filter/mgt/DefaultFilterChainManager.java |  37 ++++-
>  .../web/filter/mgt/FilterChainManager.java    |  22 +++
>  .../web/servlet/AbstractShiroFilter.java      |   1 +
>  .../IniFilterChainResolverFactoryTest.groovy  |  26 +++
>  .../web/env/IniWebEnvironmentTest.groovy      |  69 ++++++++
>  .../filter/InvalidRequestFilterTest.groovy    | 106 ++++++++++++
>  .../mgt/DefaultFilterChainManagerTest.groovy  |  52 ++++++
>  .../org/apache/shiro/web/env/FilterStub.java  |  45 ++++++
>  24 files changed, 925 insertions(+), 19 deletions(-)
>  create mode 100644 support/spring-boot/spring-boot-web-starter/src/test/groovy/org/apache/shiro/spring/boot/autoconfigure/web/ConfiguredGlobalFiltersTest.groovy
>  create mode 100644 support/spring-boot/spring-boot-web-starter/src/test/groovy/org/apache/shiro/spring/boot/autoconfigure/web/DisabledGlobalFiltersTest.groovy
>  create mode 100644 web/src/main/java/org/apache/shiro/web/filter/InvalidRequestFilter.java
>  create mode 100644 web/src/test/groovy/org/apache/shiro/web/filter/InvalidRequestFilterTest.groovy
>  create mode 100644 web/src/test/java/org/apache/shiro/web/env/FilterStub.java
> 
> --- a/support/guice/src/main/java/org/apache/shiro/guice/web/ShiroWebModule.java
> +++ b/support/guice/src/main/java/org/apache/shiro/guice/web/ShiroWebModule.java
> @@ -18,9 +18,13 @@
>   */
>  package org.apache.shiro.guice.web;
>  
> +import java.util.ArrayList;
> +import java.util.Arrays;
>  import java.util.Collection;
> +import java.util.Collections;
>  import java.util.HashMap;
>  import java.util.LinkedHashMap;
> +import java.util.List;
>  import java.util.Map;
>  
>  import javax.servlet.Filter;
> @@ -32,6 +36,7 @@
>  import org.apache.shiro.mgt.SecurityManager;
>  import org.apache.shiro.session.mgt.SessionManager;
>  import org.apache.shiro.web.env.WebEnvironment;
> +import org.apache.shiro.web.filter.InvalidRequestFilter;;
>  import org.apache.shiro.web.filter.PathMatchingFilter;
>  import org.apache.shiro.web.filter.authc.AnonymousFilter;
>  import org.apache.shiro.web.filter.authc.BasicHttpAuthenticationFilter;
> @@ -86,7 +91,8 @@
>      public static final Key<SslFilter> SSL = Key.get(SslFilter.class);
>      @SuppressWarnings({"UnusedDeclaration"})
>      public static final Key<UserFilter> USER = Key.get(UserFilter.class);
> -
> +    @SuppressWarnings({"UnusedDeclaration"})
> +    public static final Key<InvalidRequestFilter> INVALID_REQUEST = Key.get(InvalidRequestFilter.class);
>  
>      static final String NAME = "SHIRO";
>  
> @@ -123,6 +129,12 @@
>          };
>      }
>  
> +    public List<Key<? extends Filter>> globalFilters() {
> +        ArrayList<Key<? extends Filter>> filters = new ArrayList<Key<? extends Filter>>();
> +        filters.add(INVALID_REQUEST);
> +        return Collections.unmodifiableList(filters);
> +    }
> +
>      @Override
>      protected final void configureShiro() {
>          bindBeanType(TypeLiteral.get(ServletContext.class), Key.get(ServletContext.class, Names.named(NAME)));
> @@ -134,6 +146,12 @@
>  
>          this.configureShiroWeb();
>  
> +        // add default matching route if not already set
> +        if (!filterChains.containsKey("/**")) {
> +            // no config, this will add only the global filters
> +            this.addFilterChain("/**", new Key[0]);
> +        }
> +
>          setupFilterChainConfigs();
>  
>          bind(FilterChainResolver.class).toProvider(new FilterChainResolverProvider(filterChains));
> @@ -143,8 +161,15 @@
>          Map<Key<? extends PathMatchingFilter>, Map<String, String>> configs = new HashMap<Key<? extends PathMatchingFilter>, Map<String, String>>();
>  
>          for (Map.Entry<String, Key<? extends Filter>[]> filterChain : filterChains.entrySet()) {
> -            for (int i = 0; i < filterChain.getValue().length; i++) {
> -                Key<? extends Filter> key = filterChain.getValue()[i];
> +            List<Key<? extends Filter>> globalFilters = this.globalFilters();
> +            Key<? extends Filter>[] pathFilters = filterChain.getValue();
> +
> +            // merge the global filters and the path specific filters
> +            List<Key<? extends Filter>> filterConfigs = new ArrayList<Key<? extends Filter>>(globalFilters.size() + pathFilters.length);
> +            filterConfigs.addAll(globalFilters);
> +            filterConfigs.addAll(Arrays.asList(pathFilters));
> +            for (int i = 0; i < filterConfigs.size(); i++) {
> +                Key<? extends Filter> key = filterConfigs.get(i);
>                  if (key instanceof FilterConfigKey) {
>                      FilterConfigKey<? extends PathMatchingFilter> configKey = (FilterConfigKey<? extends PathMatchingFilter>) key;
>                      key = configKey.getKey();
> --- a/support/spring/src/main/java/org/apache/shiro/spring/web/ShiroFilterFactoryBean.java
> +++ b/support/spring/src/main/java/org/apache/shiro/spring/web/ShiroFilterFactoryBean.java
> @@ -25,8 +25,10 @@
>  import org.apache.shiro.util.StringUtils;
>  import org.apache.shiro.web.config.IniFilterChainResolverFactory;
>  import org.apache.shiro.web.filter.AccessControlFilter;
> +import org.apache.shiro.web.filter.InvalidRequestFilter;
>  import org.apache.shiro.web.filter.authc.AuthenticationFilter;
>  import org.apache.shiro.web.filter.authz.AuthorizationFilter;
> +import org.apache.shiro.web.filter.mgt.DefaultFilter;
>  import org.apache.shiro.web.filter.mgt.DefaultFilterChainManager;
>  import org.apache.shiro.web.filter.mgt.FilterChainManager;
>  import org.apache.shiro.web.filter.mgt.FilterChainResolver;
> @@ -41,7 +43,9 @@
>  import org.springframework.beans.factory.config.BeanPostProcessor;
>  
>  import javax.servlet.Filter;
> +import java.util.ArrayList;
>  import java.util.LinkedHashMap;
> +import java.util.List;
>  import java.util.Map;
>  
>  /**
> @@ -121,6 +125,8 @@
>  
>      private Map<String, Filter> filters;
>  
> +    private List<String> globalFilters;
> +
>      private Map<String, String> filterChainDefinitionMap; //urlPathExpression_to_comma-delimited-filter-chain-definition
>  
>      private String loginUrl;
> @@ -131,6 +137,8 @@
>  
>      public ShiroFilterFactoryBean() {
>          this.filters = new LinkedHashMap<String, Filter>();
> +        this.globalFilters = new ArrayList<String>();
> +        this.globalFilters.add(DefaultFilter.invalidRequest.name());
>          this.filterChainDefinitionMap = new LinkedHashMap<String, String>(); //order matters!
>      }
>  
> @@ -332,6 +340,14 @@
>      }
>  
>      /**
> +     * Sets the list of filters that will be executed against every request.  Defaults to the {@link InvalidRequestFilter} which will block known invalid request attacks.
> +     * @param globalFilters the list of filters to execute before specific path filters.
> +     */
> +    public void setGlobalFilters(List<String> globalFilters) {
> +        this.globalFilters = globalFilters;
> +    }
> +
> +    /**
>       * Lazily creates and returns a {@link AbstractShiroFilter} concrete instance via the
>       * {@link #createInstance} method.
>       *
> @@ -388,6 +404,9 @@
>              }
>          }
>  
> +        // set the global filters
> +        manager.setGlobalFilters(this.globalFilters);
> +
>          //build up the chains:
>          Map<String, String> chains = getFilterChainDefinitionMap();
>          if (!CollectionUtils.isEmpty(chains)) {
> @@ -398,6 +417,9 @@
>              }
>          }
>  
> +        // create the default chain, to match anything the path matching would have missed
> +        manager.createDefaultChain("/**"); // TODO this assumes ANT path matching, which might be OK here
> +
>          return manager;
>      }
>  
> --- a/web/src/main/java/org/apache/shiro/web/config/IniFilterChainResolverFactory.java
> +++ b/web/src/main/java/org/apache/shiro/web/config/IniFilterChainResolverFactory.java
> @@ -24,6 +24,7 @@
>  import org.apache.shiro.config.ReflectionBuilder;
>  import org.apache.shiro.util.CollectionUtils;
>  import org.apache.shiro.util.Factory;
> +import org.apache.shiro.web.filter.mgt.DefaultFilter;
>  import org.apache.shiro.web.filter.mgt.FilterChainManager;
>  import org.apache.shiro.web.filter.mgt.FilterChainResolver;
>  import org.apache.shiro.web.filter.mgt.PathMatchingFilterChainResolver;
> @@ -32,7 +33,9 @@
>  
>  import javax.servlet.Filter;
>  import javax.servlet.FilterConfig;
> +import java.util.Collections;
>  import java.util.LinkedHashMap;
> +import java.util.List;
>  import java.util.Map;
>  
>  /**
> @@ -49,6 +52,8 @@
>  
>      private FilterConfig filterConfig;
>  
> +    private List<String> globalFilters = Collections.singletonList(DefaultFilter.invalidRequest.name());
> +
>      private Map<String, ?> defaultBeans;
>  
>      public IniFilterChainResolverFactory() {
> @@ -72,6 +77,14 @@
>          this.filterConfig = filterConfig;
>      }
>  
> +    public List<String> getGlobalFilters() {
> +        return globalFilters;
> +    }
> +
> +    public void setGlobalFilters(List<String> globalFilters) {
> +        this.globalFilters = globalFilters;
> +    }
> +
>      protected FilterChainResolver createInstance(Ini ini) {
>          FilterChainResolver filterChainResolver = createDefaultInstance();
>          if (filterChainResolver instanceof PathMatchingFilterChainResolver) {
> @@ -122,9 +135,14 @@
>          //add the filters to the manager:
>          registerFilters(filters, manager);
>  
> +        manager.setGlobalFilters(getGlobalFilters());
> +
>          //urls section:
>          section = ini.getSection(URLS);
>          createChains(section, manager);
> +
> +        // create the default chain, to match anything the path matching would have missed
> +        manager.createDefaultChain("/**"); // TODO this assumes ANT path matching
>      }
>  
>      protected void registerFilters(Map<String, Filter> filters, FilterChainManager manager) {
> --- /dev/null
> +++ b/web/src/main/java/org/apache/shiro/web/filter/InvalidRequestFilter.java
> @@ -0,0 +1,136 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements.  See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership.  The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License.  You may obtain a copy of the License at
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied.  See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + */
> +
> +package org.apache.shiro.web.filter;
> +
> +import org.apache.shiro.web.util.WebUtils;
> +
> +import javax.servlet.ServletRequest;
> +import javax.servlet.ServletResponse;
> +import java.util.Arrays;
> +import java.util.Collections;
> +import java.util.List;
> +
> +/**
> + * A request filter that blocks malicious requests. Invalid request will respond with a 400 response code.
> + * <p>
> + * This filter checks and blocks the request if the following characters are found in the request URI:
> + * <ul>
> + *     <li>Semicolon - can be disabled by setting {@code blockSemicolon = false}</li>
> + *     <li>Backslash - can be disabled by setting {@code blockBackslash = false}</li>
> + *     <li>Non-ASCII characters - can be disabled by setting {@code blockNonAscii = false}, the ability to disable this check will be removed in future version.</li>
> + * </ul>
> + *
> + * @see <a href="https://docs.spring.io/spring-security/site/docs/current/api/org/springframework/security/web/firewall/StrictHttpFirewall.html">This class was inspired by Spring Security StrictHttpFirewall</a>
> + * @since 1.6
> + */
> +public class InvalidRequestFilter extends AccessControlFilter {
> +
> +    private static final List<String> SEMICOLON = Collections.unmodifiableList(Arrays.asList(";", "%3b", "%3B"));
> +
> +    private static final List<String> BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C"));
> +
> +    private boolean blockSemicolon = true;
> +
> +    private boolean blockBackslash = true;
> +
> +    private boolean blockNonAscii = true;
> +
> +    @Override
> +    protected boolean isAccessAllowed(ServletRequest request, ServletResponse response, Object mappedValue) throws Exception {
> +        String uri = WebUtils.toHttp(request).getRequestURI();
> +        return !containsSemicolon(uri)
> +            && !containsBackslash(uri)
> +            && !containsNonAsciiCharacters(uri);
> +    }
> +
> +    @Override
> +    protected boolean onAccessDenied(ServletRequest request, ServletResponse response) throws Exception {
> +        WebUtils.toHttp(response).sendError(400, "Invalid request");
> +        return false;
> +    }
> +
> +    private boolean containsSemicolon(String uri) {
> +        if (isBlockSemicolon()) {
> +            int length = uri.length();
> +            for (int i = 0; i < length; i++) {
> +                char c = uri.charAt(i);
> +                if (c == ';') {
> +                    return true;
> +                }
> +            }
> +        }
> +        return false;
> +    }
> +
> +    private boolean containsBackslash(String uri) {
> +        if (isBlockBackslash()) {
> +            int length = uri.length();
> +            for (int i = 0; i < length; i++) {
> +                char c = uri.charAt(i);
> +                if (c == '\\') {
> +                    return true;
> +                }
> +            }
> +        }
> +        return false;
> +    }
> +
> +    private boolean containsNonAsciiCharacters(String uri) {
> +        if (isBlockNonAscii()) {
> +            return !containsOnlyPrintableAsciiCharacters(uri);
> +        }
> +        return false;
> +    }
> +
> +    private static boolean containsOnlyPrintableAsciiCharacters(String uri) {
> +        int length = uri.length();
> +        for (int i = 0; i < length; i++) {
> +            char c = uri.charAt(i);
> +            if (c < '\u0020' || c > '\u007e') {
> +                return false;
> +            }
> +        }
> +        return true;
> +    }
> +
> +    public boolean isBlockSemicolon() {
> +        return blockSemicolon;
> +    }
> +
> +    public void setBlockSemicolon(boolean blockSemicolon) {
> +        this.blockSemicolon = blockSemicolon;
> +    }
> +
> +    public boolean isBlockBackslash() {
> +        return blockBackslash;
> +    }
> +
> +    public void setBlockBackslash(boolean blockBackslash) {
> +        this.blockBackslash = blockBackslash;
> +    }
> +
> +    public boolean isBlockNonAscii() {
> +        return blockNonAscii;
> +    }
> +
> +    public void setBlockNonAscii(boolean blockNonAscii) {
> +        this.blockNonAscii = blockNonAscii;
> +    }
> +}
> --- a/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilter.java
> +++ b/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilter.java
> @@ -19,6 +19,7 @@
>  package org.apache.shiro.web.filter.mgt;
>  
>  import org.apache.shiro.util.ClassUtils;
> +import org.apache.shiro.web.filter.InvalidRequestFilter;
>  import org.apache.shiro.web.filter.authc.*;
>  import org.apache.shiro.web.filter.authz.*;
>  import org.apache.shiro.web.filter.session.NoSessionCreationFilter;
> @@ -47,7 +48,8 @@
>      rest(HttpMethodPermissionFilter.class),
>      roles(RolesAuthorizationFilter.class),
>      ssl(SslFilter.class),
> -    user(UserFilter.class);
> +    user(UserFilter.class),
> +    invalidRequest(InvalidRequestFilter.class);
>  
>      private final Class<? extends Filter> filterClass;
>  
> --- a/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilterChainManager.java
> +++ b/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilterChainManager.java
> @@ -30,8 +30,10 @@
>  import javax.servlet.FilterChain;
>  import javax.servlet.FilterConfig;
>  import javax.servlet.ServletException;
> +import java.util.ArrayList;
>  import java.util.Collections;
>  import java.util.LinkedHashMap;
> +import java.util.List;
>  import java.util.Map;
>  import java.util.Set;
>  
> @@ -52,17 +54,21 @@
>  
>      private Map<String, Filter> filters; //pool of filters available for creating chains
>  
> +    private List<String> globalFilterNames; // list of filters to prepend to every chain
> +
>      private Map<String, NamedFilterList> filterChains; //key: chain name, value: chain
>  
>      public DefaultFilterChainManager() {
>          this.filters = new LinkedHashMap<String, Filter>();
>          this.filterChains = new LinkedHashMap<String, NamedFilterList>();
> +        this.globalFilterNames = new ArrayList<String>();
>          addDefaultFilters(false);
>      }
>  
>      public DefaultFilterChainManager(FilterConfig filterConfig) {
>          this.filters = new LinkedHashMap<String, Filter>();
>          this.filterChains = new LinkedHashMap<String, NamedFilterList>();
> +        this.globalFilterNames = new ArrayList<String>();
>          setFilterConfig(filterConfig);
>          addDefaultFilters(true);
>      }
> @@ -115,6 +121,17 @@
>          addFilter(name, filter, init, true);
>      }
>  
> +    public void createDefaultChain(String chainName) {
> +        // only create the defaultChain if we don't have a chain with this name already
> +        // (the global filters will already be in that chain)
> +        if (!getChainNames().contains(chainName) && !CollectionUtils.isEmpty(globalFilterNames)) {
> +            // add each of global filters
> +            for (String filterName : globalFilterNames) {
> +                addToChain(chainName, filterName);
> +            }
> +        }
> +    }
> +
>      public void createChain(String chainName, String chainDefinition) {
>          if (!StringUtils.hasText(chainName)) {
>              throw new NullPointerException("chainName cannot be null or empty.");
> @@ -124,7 +141,14 @@
>          }
>  
>          if (log.isDebugEnabled()) {
> -            log.debug("Creating chain [" + chainName + "] from String definition [" + chainDefinition + "]");
> +            log.debug("Creating chain [" + chainName + "] with global filters " + globalFilterNames + " and from String definition [" + chainDefinition + "]");
> +        }
> +
> +        // first add each of global filters
> +        if (!CollectionUtils.isEmpty(globalFilterNames)) {
> +            for (String filterName : globalFilterNames) {
> +                addToChain(chainName, filterName);
> +            }
>          }
>  
>          //parse the value by tokenizing it to get the resulting filter-specific config entries
> @@ -273,6 +297,21 @@
>          chain.add(filter);
>      }
>  
> +    public void setGlobalFilters(List<String> globalFilterNames) throws ConfigurationException {
> +        // validate each filter name
> +        if (!CollectionUtils.isEmpty(globalFilterNames)) {
> +            for (String filterName : globalFilterNames) {
> +                Filter filter = filters.get(filterName);
> +                if (filter == null) {
> +                    throw new ConfigurationException("There is no filter with name '" + filterName +
> +                                                     "' to apply to the global filters in the pool of available Filters.  Ensure a " +
> +                                                     "filter with that name/path has first been registered with the addFilter method(s).");
> +                }
> +                this.globalFilterNames.add(filterName);
> +            }
> +        }
> +    }
> +
>      protected void applyChainConfig(String chainName, Filter filter, String chainSpecificFilterConfig) {
>          if (log.isDebugEnabled()) {
>              log.debug("Attempting to apply path [" + chainName + "] to filter [" + filter + "] " +
> --- a/web/src/main/java/org/apache/shiro/web/filter/mgt/FilterChainManager.java
> +++ b/web/src/main/java/org/apache/shiro/web/filter/mgt/FilterChainManager.java
> @@ -22,6 +22,7 @@
>  
>  import javax.servlet.Filter;
>  import javax.servlet.FilterChain;
> +import java.util.List;
>  import java.util.Map;
>  import java.util.Set;
>  
> @@ -165,6 +166,14 @@
>      void createChain(String chainName, String chainDefinition);
>  
>      /**
> +     * Creates a chain that should match any non-matched request paths, typically {@code /**} assuming an {@link AntPathMatcher} I used.
> +     * @param chainName The name of the chain to create, likely {@code /**}.
> +     * @since 1.6
> +     * @see org.apache.shiro.lang.util.AntPathMatcher AntPathMatcher
> +     */
> +    void createDefaultChain(String chainName);
> +
> +    /**
>       * Adds (appends) a filter to the filter chain identified by the given {@code chainName}.  If there is no chain
>       * with the given name, a new one is created and the filter will be the first in the chain.
>       *
> @@ -195,4 +204,17 @@
>       *                                  interface).
>       */
>      void addToChain(String chainName, String filterName, String chainSpecificFilterConfig) throws ConfigurationException;
> +
> +    /**
> +     * Configures the set of named filters that will match all paths.  These filters will match BEFORE explicitly
> +     * configured filter chains i.e. by calling {@link #createChain(String, String)}, {@link #addToChain(String, String)}, etc.
> +     * <br>
> +     * <strong>Filters configured in this list wll apply to ALL requests.</strong>
> +     *
> +     * @param globalFilterNames         the list of filter names to match ALL paths.
> +     * @throws ConfigurationException   if one of the filter names is invalid and cannot be loaded from the set of
> +     *                                  configured filters {@link #getFilters()}}.
> +     * @since 1.6
> +     */
> +    void setGlobalFilters(List<String> globalFilterNames) throws ConfigurationException;
>  }


-- 
Roberto C. Sánchez
http://people.connexer.com/~roberto
http://www.connexer.com

Re: Request for assistance to backport CVE-2020-13933 fix

Posted by "Roberto C. Sánchez" <ro...@debian.org>.
Hi Brian, Ben, et al.,

I have what I believe to be a correct backport of commit dc194fc977ab to
address CVE-2020-13933.  The adaptations/changes include:

- replacing the diamond operator <> with type specifications (the 1.3.x
  build uses source compatibility level 1.6)
- replacing stream constructs with loops
- omitting changes to non-existent tests
- adjusting for the absence of the FilterConfig type (which seems to
  have been introduced in 1.4)

If someone familiar with the code, the vulnerability, and the fix could
review the attached patch that would be very much appreciated.  Also, it
would be quite helpful if there was a reproducer or a description of how
to exploit the vulnerability to help with validating the fix.  If that
cannot be disclosed, then perhaps someone could confirm that this patch
does in fact address the vulnerability; that would be helpful.

Next I will start working on CVE-2020-17510 and CVE-2020-17523.  Once I
have those sorted out for the existing 1.3.x packages I will turn my
attention to seeing about updating to a newer upstream release (which
could be a lengthy process).

Regards,

-Roberto

On Wed, Mar 31, 2021 at 03:21:55PM -0400, Roberto C. S�nchez wrote:
> Hi Brian,
> 
> Thanks for your help.  I am working to backport dc194fc977ab to address
> CVE-2020-13933 and after that I will move on to the fixes for
> CVE-2020-17510 and CVE-2020-17523.
> 
> As far as the maintainability of a 1.3.x package, upgrading to a newer
> version is not an option for two reasons.  First, I am working on
> addressing these vulnerabilities in Debian Stretch, which is in the LTS
> stage of its lifecycle, making an update to a new upstream release very
> unlikely.  Second, the shiro package in Debian was last updated about 2
> years ago and even in Debian unstable 1.3.x is the highest available
> version.  Any update to a new upstream version would need to start in
> the unstable distribution.
> 
> As an additional complication, the activemq package in Debian depends on
> the shiro package, so a shiro update would need to be coordinated in a
> that ensures it doesn't break activemq.
> 
> That said, I will bring the issue up to see if I can get an update
> started so that at the least the next Debian stable release has
> something more recent.
> 
> Regards,
> 
> -Roberto
> 
> On Tue, Mar 16, 2021 at 05:50:50PM -0400, Brian Demers wrote:
> > Hey Roberto,
> > 
> > Sorry about the delay on this one, I originally thought we had answered
> > your question.
> > 
> > The commit you are looking for is
> > https://github.com/apache/shiro/commit/dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d
> > 
> > If you are maintaining a 1.3.x package this is going to become more
> > difficult, is it possible to deprecate it and move to a recent version?
> > 
> > 
> > 
> > On Sat, Jan 30, 2021 at 4:45 PM Roberto C. S�nchez <ro...@debian.org>
> > wrote:
> > 
> > > Bump.
> > >
> > > On Tue, Dec 22, 2020 at 09:30:47AM -0500, Roberto C. S�nchez wrote:
> > > > On Mon, Dec 21, 2020 at 09:33:44PM +0100, Benjamin Marwell wrote:
> > > > > Hi Roberto,
> > > > >
> > > > > after talking to the PMC chair, I can give you three commit links.
> > > > >
> > > > >
> > > https://github.com/apache/shiro/commit/042c59356cc6442345a9f935aed3e7603cb4dfad
> > > > >
> > > https://github.com/apache/shiro/commit/5b1add9a4c4ed046b52cf2132ed0f264a22caf1d
> > > > >
> > > https://github.com/apache/shiro/commit/1b9d8d99cd6d50d7114916508a13677a0fe6f345
> > > > >
> > > > > I guess it is quite obvious what is inside these commits.
> > > > >
> > > > Hi Ben,
> > > >
> > > > This commits seem to have been made after the 1.6.0 release and before
> > > > the 1.7.0 release.  My belief is that they address CVE-2020-17510.  Can
> > > > you tell me if dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d is the commit
> > > > that addresses CVE-2020-13933?  Are there other commits that go along
> > > > with it to completely remedy CVE-2020-13933?
> > > >
> > > > Regards,
> > > >
> > > > -Roberto
> > > >
> > > > --
> > > > Roberto C. S�nchez
> > >
> > > --
> > > Roberto C. S�nchez
> > >
> 
> -- 
> Roberto C. S�nchez
> http://people.connexer.com/~roberto
> http://www.connexer.com

-- 
Roberto C. S�nchez
http://people.connexer.com/~roberto
http://www.connexer.com

Re: Request for assistance to backport CVE-2020-13933 fix

Posted by "Roberto C. Sánchez" <ro...@debian.org>.
That is very helpful.  I will bring it up with the team responsible for
the stable updates and those reponsible for the activemq and shiro
packages to see if we can improve the situation.

Regards,

-Roberto

On Wed, Mar 31, 2021 at 10:18:59PM +0200, Francois Papon wrote:
> As I see in the activemq repo:
> 
> amq 5.14.3 => shiro 1.2.4
> 
> amq 5.15.8 => shiro 1.2.6
> 
> amq 5.16.1 => shiro 1.7.0
> 
> the latest 5.15.x is the 5.15.14 and it's using shiro 1.7.0
> 
> may be it could be an option to upgrade stable to 5.15.14
> 
> regards,
> 
> François
> fpapon@apache.org
> 
> Le 31/03/2021 à 22:01, Roberto C. Sánchez a écrit :
> > Hi François,
> >
> > Debian currently has activemq versions as follows:
> >
> > unstable/testing: 5.16.1
> > stable: 5.15.8
> > old stable (LTS): 5.14.3
> >
> > The most recent update (5.16.1) was uploaded near the beginning of
> > March.
> >
> > An update of shiro in unstable would only need to be concerned with the
> > activemq in unstable.  However, an update to shiro in stable or old
> > stable would need to account for the older activemq versions there.
> >
> > Regards,
> >
> > -Roberto
> >
> > On Wed, Mar 31, 2021 at 09:48:33PM +0200, Francois Papon wrote:
> >> Hi Roberto,
> >>
> >> Which version of activemq are you using in the debian package?
> >>
> >> regards,
> >>
> >> François
> >> fpapon@apache.org
> >>

-- 
Roberto C. Sánchez

Re: Request for assistance to backport CVE-2020-13933 fix

Posted by Francois Papon <fr...@openobject.fr>.
As I see in the activemq repo:

amq 5.14.3 => shiro 1.2.4

amq 5.15.8 => shiro 1.2.6

amq 5.16.1 => shiro 1.7.0

the latest 5.15.x is the 5.15.14 and it's using shiro 1.7.0

may be it could be an option to upgrade stable to 5.15.14

regards,

François
fpapon@apache.org

Le 31/03/2021 à 22:01, Roberto C. Sánchez a écrit :
> Hi François,
>
> Debian currently has activemq versions as follows:
>
> unstable/testing: 5.16.1
> stable: 5.15.8
> old stable (LTS): 5.14.3
>
> The most recent update (5.16.1) was uploaded near the beginning of
> March.
>
> An update of shiro in unstable would only need to be concerned with the
> activemq in unstable.  However, an update to shiro in stable or old
> stable would need to account for the older activemq versions there.
>
> Regards,
>
> -Roberto
>
> On Wed, Mar 31, 2021 at 09:48:33PM +0200, Francois Papon wrote:
>> Hi Roberto,
>>
>> Which version of activemq are you using in the debian package?
>>
>> regards,
>>
>> François
>> fpapon@apache.org
>>

Re: Request for assistance to backport CVE-2020-13933 fix

Posted by "Roberto C. Sánchez" <ro...@debian.org>.
Hi François,

Debian currently has activemq versions as follows:

unstable/testing: 5.16.1
stable: 5.15.8
old stable (LTS): 5.14.3

The most recent update (5.16.1) was uploaded near the beginning of
March.

An update of shiro in unstable would only need to be concerned with the
activemq in unstable.  However, an update to shiro in stable or old
stable would need to account for the older activemq versions there.

Regards,

-Roberto

On Wed, Mar 31, 2021 at 09:48:33PM +0200, Francois Papon wrote:
> Hi Roberto,
> 
> Which version of activemq are you using in the debian package?
> 
> regards,
> 
> François
> fpapon@apache.org
> 

-- 
Roberto C. Sánchez

Re: Request for assistance to backport CVE-2020-13933 fix

Posted by Francois Papon <fr...@openobject.fr>.
Hi Roberto,

Which version of activemq are you using in the debian package?

regards,

François
fpapon@apache.org

Le 31/03/2021 à 21:21, Roberto C. Sánchez a écrit :
> Hi Brian,
>
> Thanks for your help.  I am working to backport dc194fc977ab to address
> CVE-2020-13933 and after that I will move on to the fixes for
> CVE-2020-17510 and CVE-2020-17523.
>
> As far as the maintainability of a 1.3.x package, upgrading to a newer
> version is not an option for two reasons.  First, I am working on
> addressing these vulnerabilities in Debian Stretch, which is in the LTS
> stage of its lifecycle, making an update to a new upstream release very
> unlikely.  Second, the shiro package in Debian was last updated about 2
> years ago and even in Debian unstable 1.3.x is the highest available
> version.  Any update to a new upstream version would need to start in
> the unstable distribution.
>
> As an additional complication, the activemq package in Debian depends on
> the shiro package, so a shiro update would need to be coordinated in a
> that ensures it doesn't break activemq.
>
> That said, I will bring the issue up to see if I can get an update
> started so that at the least the next Debian stable release has
> something more recent.
>
> Regards,
>
> -Roberto
>
> On Tue, Mar 16, 2021 at 05:50:50PM -0400, Brian Demers wrote:
>> Hey Roberto,
>>
>> Sorry about the delay on this one, I originally thought we had answered
>> your question.
>>
>> The commit you are looking for is
>> https://github.com/apache/shiro/commit/dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d
>>
>> If you are maintaining a 1.3.x package this is going to become more
>> difficult, is it possible to deprecate it and move to a recent version?
>>
>>
>>
>> On Sat, Jan 30, 2021 at 4:45 PM Roberto C. Sánchez <ro...@debian.org>
>> wrote:
>>
>>> Bump.
>>>
>>> On Tue, Dec 22, 2020 at 09:30:47AM -0500, Roberto C. Sánchez wrote:
>>>> On Mon, Dec 21, 2020 at 09:33:44PM +0100, Benjamin Marwell wrote:
>>>>> Hi Roberto,
>>>>>
>>>>> after talking to the PMC chair, I can give you three commit links.
>>>>>
>>>>>
>>> https://github.com/apache/shiro/commit/042c59356cc6442345a9f935aed3e7603cb4dfad
>>> https://github.com/apache/shiro/commit/5b1add9a4c4ed046b52cf2132ed0f264a22caf1d
>>> https://github.com/apache/shiro/commit/1b9d8d99cd6d50d7114916508a13677a0fe6f345
>>>>> I guess it is quite obvious what is inside these commits.
>>>>>
>>>> Hi Ben,
>>>>
>>>> This commits seem to have been made after the 1.6.0 release and before
>>>> the 1.7.0 release.  My belief is that they address CVE-2020-17510.  Can
>>>> you tell me if dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d is the commit
>>>> that addresses CVE-2020-13933?  Are there other commits that go along
>>>> with it to completely remedy CVE-2020-13933?
>>>>
>>>> Regards,
>>>>
>>>> -Roberto
>>>>
>>>> --
>>>> Roberto C. Sánchez
>>> --
>>> Roberto C. Sánchez
>>>

Re: Request for assistance to backport CVE-2020-13933 fix

Posted by "Roberto C. Sánchez" <ro...@debian.org>.
Hi Brian,

Thanks for your help.  I am working to backport dc194fc977ab to address
CVE-2020-13933 and after that I will move on to the fixes for
CVE-2020-17510 and CVE-2020-17523.

As far as the maintainability of a 1.3.x package, upgrading to a newer
version is not an option for two reasons.  First, I am working on
addressing these vulnerabilities in Debian Stretch, which is in the LTS
stage of its lifecycle, making an update to a new upstream release very
unlikely.  Second, the shiro package in Debian was last updated about 2
years ago and even in Debian unstable 1.3.x is the highest available
version.  Any update to a new upstream version would need to start in
the unstable distribution.

As an additional complication, the activemq package in Debian depends on
the shiro package, so a shiro update would need to be coordinated in a
that ensures it doesn't break activemq.

That said, I will bring the issue up to see if I can get an update
started so that at the least the next Debian stable release has
something more recent.

Regards,

-Roberto

On Tue, Mar 16, 2021 at 05:50:50PM -0400, Brian Demers wrote:
> Hey Roberto,
> 
> Sorry about the delay on this one, I originally thought we had answered
> your question.
> 
> The commit you are looking for is
> https://github.com/apache/shiro/commit/dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d
> 
> If you are maintaining a 1.3.x package this is going to become more
> difficult, is it possible to deprecate it and move to a recent version?
> 
> 
> 
> On Sat, Jan 30, 2021 at 4:45 PM Roberto C. Sánchez <ro...@debian.org>
> wrote:
> 
> > Bump.
> >
> > On Tue, Dec 22, 2020 at 09:30:47AM -0500, Roberto C. Sánchez wrote:
> > > On Mon, Dec 21, 2020 at 09:33:44PM +0100, Benjamin Marwell wrote:
> > > > Hi Roberto,
> > > >
> > > > after talking to the PMC chair, I can give you three commit links.
> > > >
> > > >
> > https://github.com/apache/shiro/commit/042c59356cc6442345a9f935aed3e7603cb4dfad
> > > >
> > https://github.com/apache/shiro/commit/5b1add9a4c4ed046b52cf2132ed0f264a22caf1d
> > > >
> > https://github.com/apache/shiro/commit/1b9d8d99cd6d50d7114916508a13677a0fe6f345
> > > >
> > > > I guess it is quite obvious what is inside these commits.
> > > >
> > > Hi Ben,
> > >
> > > This commits seem to have been made after the 1.6.0 release and before
> > > the 1.7.0 release.  My belief is that they address CVE-2020-17510.  Can
> > > you tell me if dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d is the commit
> > > that addresses CVE-2020-13933?  Are there other commits that go along
> > > with it to completely remedy CVE-2020-13933?
> > >
> > > Regards,
> > >
> > > -Roberto
> > >
> > > --
> > > Roberto C. Sánchez
> >
> > --
> > Roberto C. Sánchez
> >

-- 
Roberto C. Sánchez
http://people.connexer.com/~roberto
http://www.connexer.com

Re: Request for assistance to backport CVE-2020-13933 fix

Posted by Brian Demers <br...@gmail.com>.
Hey Roberto,

Sorry about the delay on this one, I originally thought we had answered
your question.

The commit you are looking for is
https://github.com/apache/shiro/commit/dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d

If you are maintaining a 1.3.x package this is going to become more
difficult, is it possible to deprecate it and move to a recent version?



On Sat, Jan 30, 2021 at 4:45 PM Roberto C. Sánchez <ro...@debian.org>
wrote:

> Bump.
>
> On Tue, Dec 22, 2020 at 09:30:47AM -0500, Roberto C. Sánchez wrote:
> > On Mon, Dec 21, 2020 at 09:33:44PM +0100, Benjamin Marwell wrote:
> > > Hi Roberto,
> > >
> > > after talking to the PMC chair, I can give you three commit links.
> > >
> > >
> https://github.com/apache/shiro/commit/042c59356cc6442345a9f935aed3e7603cb4dfad
> > >
> https://github.com/apache/shiro/commit/5b1add9a4c4ed046b52cf2132ed0f264a22caf1d
> > >
> https://github.com/apache/shiro/commit/1b9d8d99cd6d50d7114916508a13677a0fe6f345
> > >
> > > I guess it is quite obvious what is inside these commits.
> > >
> > Hi Ben,
> >
> > This commits seem to have been made after the 1.6.0 release and before
> > the 1.7.0 release.  My belief is that they address CVE-2020-17510.  Can
> > you tell me if dc194fc977ab6cfbf3c1ecb085e2bac5db14af6d is the commit
> > that addresses CVE-2020-13933?  Are there other commits that go along
> > with it to completely remedy CVE-2020-13933?
> >
> > Regards,
> >
> > -Roberto
> >
> > --
> > Roberto C. Sánchez
>
> --
> Roberto C. Sánchez
>