Skip to content

Commit

Permalink
Merge e7834d4 into 8d0382c
Browse files Browse the repository at this point in the history
  • Loading branch information
yasserzamani committed Jan 19, 2022
2 parents 8d0382c + e7834d4 commit 80c14fb
Show file tree
Hide file tree
Showing 52 changed files with 1,686 additions and 265 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker;
import com.opensymphony.xwork2.DefaultTextProvider;
import com.opensymphony.xwork2.DefaultUnknownHandlerManager;
import com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsChecker;
import com.opensymphony.xwork2.security.ExcludedPatternsChecker;
import com.opensymphony.xwork2.FileManager;
import com.opensymphony.xwork2.FileManagerFactory;
Expand Down Expand Up @@ -88,6 +89,7 @@
import com.opensymphony.xwork2.ognl.accessor.XWorkListPropertyAccessor;
import com.opensymphony.xwork2.ognl.accessor.XWorkMapPropertyAccessor;
import com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor;
import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker;
import com.opensymphony.xwork2.util.CompoundRoot;
import com.opensymphony.xwork2.LocalizedTextProvider;
import com.opensymphony.xwork2.util.StrutsLocalizedTextProvider;
Expand Down Expand Up @@ -215,6 +217,8 @@ public void register(ContainerBuilder builder, LocatableProperties props)

.factory(ExcludedPatternsChecker.class, DefaultExcludedPatternsChecker.class, Scope.PROTOTYPE)
.factory(AcceptedPatternsChecker.class, DefaultAcceptedPatternsChecker.class, Scope.PROTOTYPE)
.factory(NotExcludedAcceptedPatternsChecker.class, DefaultNotExcludedAcceptedPatternsChecker.class
, Scope.SINGLETON)

.factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON)
;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import com.opensymphony.xwork2.XWorkConstants;
import com.opensymphony.xwork2.config.entities.ActionConfig;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.security.AcceptedPatternsChecker;
import com.opensymphony.xwork2.security.ExcludedPatternsChecker;
import com.opensymphony.xwork2.util.ClearableValueStack;
import com.opensymphony.xwork2.util.Evaluated;
import com.opensymphony.xwork2.LocalizedTextProvider;
Expand Down Expand Up @@ -100,6 +102,9 @@ public class AliasInterceptor extends AbstractInterceptor {
protected LocalizedTextProvider localizedTextProvider;
protected boolean devMode = false;

private ExcludedPatternsChecker excludedPatterns;
private AcceptedPatternsChecker acceptedPatterns;

@Inject(XWorkConstants.DEV_MODE)
public void setDevMode(String mode) {
this.devMode = Boolean.parseBoolean(mode);
Expand All @@ -115,6 +120,16 @@ public void setLocalizedTextProvider(LocalizedTextProvider localizedTextProvider
this.localizedTextProvider = localizedTextProvider;
}

@Inject
public void setExcludedPatterns(ExcludedPatternsChecker excludedPatterns) {
this.excludedPatterns = excludedPatterns;
}

@Inject
public void setAcceptedPatterns(AcceptedPatternsChecker acceptedPatterns) {
this.acceptedPatterns = acceptedPatterns;
}

/**
* <p>
* Sets the name of the action parameter to look for the alias map.
Expand Down Expand Up @@ -145,7 +160,7 @@ public void setAliasesKey(String aliasesKey) {
ValueStack stack = ac.getValueStack();
Object obj = stack.findValue(aliasExpression);

if (obj != null && obj instanceof Map) {
if (obj instanceof Map) {
//get secure stack
ValueStack newStack = valueStackFactory.createValueStack(stack);
boolean clearableStack = newStack instanceof ClearableValueStack;
Expand All @@ -167,7 +182,13 @@ public void setAliasesKey(String aliasesKey) {
for (Object o : aliases.entrySet()) {
Map.Entry entry = (Map.Entry) o;
String name = entry.getKey().toString();
if (isNotAcceptableExpression(name)) {
continue;
}
String alias = (String) entry.getValue();
if (isNotAcceptableExpression(alias)) {
continue;
}
Evaluated value = new Evaluated(stack.findValue(name));
if (!value.isDefined()) {
// workaround
Expand Down Expand Up @@ -206,5 +227,65 @@ public void setAliasesKey(String aliasesKey) {

return invocation.invoke();
}


protected boolean isAccepted(String paramName) {
AcceptedPatternsChecker.IsAccepted result = acceptedPatterns.isAccepted(paramName);
if (result.isAccepted()) {
return true;
}

LOG.warn("Parameter [{}] didn't match accepted pattern [{}]! See Accepted / Excluded patterns at\n" +
"https://struts.apache.org/security/#accepted--excluded-patterns",
paramName, result.getAcceptedPattern());

return false;
}

protected boolean isExcluded(String paramName) {
ExcludedPatternsChecker.IsExcluded result = excludedPatterns.isExcluded(paramName);
if (!result.isExcluded()) {
return false;
}

LOG.warn("Parameter [{}] matches excluded pattern [{}]! See Accepted / Excluded patterns at\n" +
"https://struts.apache.org/security/#accepted--excluded-patterns",
paramName, result.getExcludedPattern());

return true;
}

/**
* Checks if expression contains vulnerable code
*
* @param expression of interceptor
* @return true|false
*/
protected boolean isNotAcceptableExpression(String expression) {
return isExcluded(expression) || !isAccepted(expression);
}

/**
* Sets a comma-delimited list of regular expressions to match
* parameters that are allowed in the parameter map (aka whitelist).
* <p>
* Don't change the default unless you know what you are doing in terms
* of security implications.
* </p>
*
* @param commaDelim A comma-delimited list of regular expressions
*/
public void setAcceptParamNames(String commaDelim) {
acceptedPatterns.setAcceptedPatterns(commaDelim);
}

/**
* Sets a comma-delimited list of regular expressions to match
* parameters that should be removed from the parameter map.
*
* @param commaDelim A comma-delimited list of regular expressions
*/
public void setExcludeParams(String commaDelim) {
excludedPatterns.setExcludedPatterns(commaDelim);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public class MockResult implements Result {

public static final String DEFAULT_PARAM = "foo";

private ActionInvocation invocation;

@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -41,7 +43,7 @@ public boolean equals(Object o) {
}

public void execute(ActionInvocation invocation) throws Exception {
// no op
this.invocation = invocation;
}

@Override
Expand All @@ -53,4 +55,7 @@ public void setFoo(String foo) {
// no op
}

public ActionInvocation getInvocation() {
return invocation;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import java.util.regex.Pattern;

/**
* Used across different interceptors to check if given string matches one of the excluded patterns.
* Used across different interceptors to check if given string matches one of the accepted patterns.
*/
public interface AcceptedPatternsChecker {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* 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 com.opensymphony.xwork2.security;

import com.opensymphony.xwork2.inject.Inject;

import java.util.Set;
import java.util.regex.Pattern;

public class DefaultNotExcludedAcceptedPatternsChecker implements NotExcludedAcceptedPatternsChecker {
private ExcludedPatternsChecker excludedPatterns;
private AcceptedPatternsChecker acceptedPatterns;


@Inject
public void setExcludedPatterns(ExcludedPatternsChecker excludedPatterns) {
this.excludedPatterns = excludedPatterns;
}

@Inject
public void setAcceptedPatterns(AcceptedPatternsChecker acceptedPatterns) {
this.acceptedPatterns = acceptedPatterns;
}

@Override
public IsAllowed isAllowed(String value) {
IsExcluded isExcluded = isExcluded(value);
if (isExcluded.isExcluded()) {
return IsAllowed.no(isExcluded.getExcludedPattern());
}

IsAccepted isAccepted = isAccepted(value);
if (!isAccepted.isAccepted()) {
return IsAllowed.no(isAccepted.getAcceptedPattern());
}

return IsAllowed.yes(isAccepted.getAcceptedPattern());
}

@Override
public IsAccepted isAccepted(String value) {
return acceptedPatterns.isAccepted(value);
}

@Override
public void setAcceptedPatterns(String commaDelimitedPatterns) {
acceptedPatterns.setAcceptedPatterns(commaDelimitedPatterns);
}

@Override
public void setAcceptedPatterns(String[] patterns) {
acceptedPatterns.setAcceptedPatterns(patterns);
}

@Override
public void setAcceptedPatterns(Set<String> patterns) {
acceptedPatterns.setAcceptedPatterns(patterns);
}

@Override
public Set<Pattern> getAcceptedPatterns() {
return acceptedPatterns.getAcceptedPatterns();
}

@Override
public IsExcluded isExcluded(String value) {
return excludedPatterns.isExcluded(value);
}

@Override
public void setExcludedPatterns(String commaDelimitedPatterns) {
excludedPatterns.setExcludedPatterns(commaDelimitedPatterns);
}

@Override
public void setExcludedPatterns(String[] patterns) {
excludedPatterns.setExcludedPatterns(patterns);
}

@Override
public void setExcludedPatterns(Set<String> patterns) {
excludedPatterns.setExcludedPatterns(patterns);
}

@Override
public Set<Pattern> getExcludedPatterns() {
return excludedPatterns.getExcludedPatterns();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* 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 com.opensymphony.xwork2.security;

/**
* Used across different places to check if given string is not excluded and is accepted
* @see <a href="https://securitylab.github.com/research/apache-struts-double-evaluation/">here</a>
* @since 2.5.27
*/
public interface NotExcludedAcceptedPatternsChecker extends ExcludedPatternsChecker, AcceptedPatternsChecker {

/**
* Checks if value doesn't match excluded pattern and matches accepted pattern
*
* @param value to check
* @return object containing result of matched pattern and pattern itself
*/
IsAllowed isAllowed(String value);

final class IsAllowed {

private final boolean allowed;
private final String allowedPattern;

public static IsAllowed yes(String allowedPattern) {
return new IsAllowed(true, allowedPattern);
}

public static IsAllowed no(String allowedPattern) {
return new IsAllowed(false, allowedPattern);
}

private IsAllowed(boolean allowed, String allowedPattern) {
this.allowed = allowed;
this.allowedPattern = allowedPattern;
}

public boolean isAllowed() {
return allowed;
}

public String getAllowedPattern() {
return allowedPattern;
}

@Override
public String toString() {
return "IsAllowed { " +
"allowed=" + allowed +
", allowedPattern=" + allowedPattern +
" }";
}
}
}
1 change: 1 addition & 0 deletions core/src/main/java/org/apache/struts2/StrutsConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ public final class StrutsConstants {
/** Dedicated services to check if passed string is excluded/accepted */
public static final String STRUTS_EXCLUDED_PATTERNS_CHECKER = "struts.excludedPatterns.checker";
public static final String STRUTS_ACCEPTED_PATTERNS_CHECKER = "struts.acceptedPatterns.checker";
public static final String STRUTS_NOT_EXCLUDED_ACCEPTED_PATTERNS_CHECKER = "struts.notExcludedAcceptedPatterns.checker";

/** Constant is used to override framework's default excluded patterns */
public static final String STRUTS_OVERRIDE_EXCLUDED_PATTERNS = "struts.override.excludedPatterns";
Expand Down

0 comments on commit 80c14fb

Please sign in to comment.