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/05/05 01:15:50 UTC
Re: Request for assistance to backport CVE-2020-13933 fix
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>.
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