Commit 2e158366 authored by cdanger's avatar cdanger

- Fixed security issues reported by find-sec-bugs

parent fe9fe507
......@@ -6,6 +6,9 @@ All notable changes to this project are documented in this file following the [K
### Changed
- Parent project version: 4.0.0 -> 4.1.0 => Saxon-HE dependency version 9.7.0-11 -> 9.7.0-14
### Fixed
- Security issues reported by findsecbugs plugin
## 8.0.0
### Added
......
<?xml version="1.0"?>
<!--
This file contains some false positive bugs detected by Findbugs. Their
false positive nature has been analyzed individually and they have been
put here to instruct Findbugs to ignore them.
-->
<FindBugsFilter>
<Match>
<Class name="org.ow2.authzforce.core.pdp.api.value.XPathValue" />
<Bug pattern="DESERIALIZATION_GADGET" />
</Match>
</FindBugsFilter>
\ No newline at end of file
......@@ -60,8 +60,7 @@
<groupId>org.owasp</groupId>
<artifactId>dependency-check-maven</artifactId>
<configuration>
<!-- The plugin has numerous issues with version matching, which triggers false positives so we need a "suppresion" file for those.
More info: https://github.com/jeremylong/DependencyCheck/issues -->
<!-- The plugin has numerous issues with version matching, which triggers false positives so we need a "suppresion" file for those. More info: https://github.com/jeremylong/DependencyCheck/issues -->
<suppressionFile>owasp-dependency-check-suppression.xml</suppressionFile>
</configuration>
<executions>
......@@ -91,6 +90,9 @@
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>findbugs-maven-plugin</artifactId>
<configuration>
<excludeFilterFile>findbugs-exclude-filter.xml</excludeFilterFile>
</configuration>
<executions>
<execution>
<phase>verify</phase>
......
......@@ -47,6 +47,17 @@ import com.google.common.collect.Sets;
*/
public final class HashCollections
{
/*
* Arbitrary limit of 1000 characters is there to mitigate Regex DoS attack
*/
// private static final String JAVA_CLASS_NAME_IDENTIFIER_PART_ASCII_ONLY_REGEX = "[a-zA-Z_$][a-zA-Z\\d_$]{0,1000}";
/*
* Arbitrary limit of 100 repetitions of class name parts is there to mitigate Regex DoS attack
*/
// private static final Pattern JAVA_CLASS_NAME_ASCII_ONLY_PATTERN = Pattern.compile("(" + JAVA_CLASS_NAME_IDENTIFIER_PART_ASCII_ONLY_REGEX + "\\.){0,100}"
// + JAVA_CLASS_NAME_IDENTIFIER_PART_ASCII_ONLY_REGEX);
/**
* Name of system property for setting the {@link HashCollectionFactory} implementation class. Default: {@link DefaultHashCollectionFactory}.
*/
......@@ -58,8 +69,8 @@ public final class HashCollections
static
{
final String hashCollectionFactoryClassName = System.getProperty(HASH_COLLECTION_FACTORY_SYSTEM_PROPERTY_NAME);
if (hashCollectionFactoryClassName == null)
final String unvalidatedHashCollectionFactoryClassName = System.getProperty(HASH_COLLECTION_FACTORY_SYSTEM_PROPERTY_NAME);
if (unvalidatedHashCollectionFactoryClassName == null)
{
LOGGER.debug("System property '{}' not set -> using {} as (default) implementation of {}", HASH_COLLECTION_FACTORY_SYSTEM_PROPERTY_NAME, DefaultHashCollectionFactory.class,
HashCollectionFactory.class);
......@@ -68,13 +79,30 @@ public final class HashCollections
else
{
// System property set
/*
* Validate characters
*/
// if (!JAVA_CLASS_NAME_ASCII_ONLY_PATTERN.matcher(hashCollectionFactoryClassName).matches())
// {
// throw new RuntimeException(
// "Error instantiating "
// + HashCollectionFactory.class
// + " from value of system property '"
// + HASH_COLLECTION_FACTORY_SYSTEM_PROPERTY_NAME
// + "': invalid class name (expected to contain only alphanumeric characters, underscore or dollar sign '$', not too big, and to be valid according to the Java Language Specification)");
// }
/*
* Must-be one-line only -> remove CRLF -> prevent CRLF log injection
*/
final String hashCollectionFactoryClassName = unvalidatedHashCollectionFactoryClassName.split("\\r?\\n", 1)[0];
LOGGER.debug("System property '{}' set to '{}'", HASH_COLLECTION_FACTORY_SYSTEM_PROPERTY_NAME, hashCollectionFactoryClassName);
try
{
final Class<?> hashCollectionFactoryClass = Class.forName(hashCollectionFactoryClassName);
final Object obj = hashCollectionFactoryClass.newInstance();
FACTORY = HashCollectionFactory.class.cast(obj);
LOGGER.debug("Set {} as implementation of {}", hashCollectionFactoryClass, HashCollectionFactory.class);
LOGGER.debug("Set {} as implementation of {}", hashCollectionFactoryClassName, HashCollectionFactory.class);
}
catch (final ClassCastException | ClassNotFoundException | InstantiationException | IllegalAccessException e)
{
......
/**
* Copyright (C) 2012-2016 Thales Services SAS.
*
* This file is part of AuthZForce CE.
*
* AuthZForce CE is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* AuthZForce CE is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with AuthZForce CE. If not, see <http://www.gnu.org/licenses/>.
*/
package org.ow2.authzforce.core.pdp.api;
import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
/**
* Utilities for String validation, sanitizing, transformation, etc.
*
*/
public final class StringUtils
{
private StringUtils()
{
}
/**
* Replace CRLF characters with "&lt;NEWLINE&gt"; in {@code input.toString()} to prevent CRLF injection
*
* @param input
* input object
* @return encoded string
*/
public static String sanitizeForLogging(final Object input)
{
try
{
return URLEncoder.encode(input.toString(), "UTF-8");
}
catch (final UnsupportedEncodingException e)
{
// UTF-8 is supported
return null;
}
}
}
......@@ -21,6 +21,7 @@ package org.ow2.authzforce.core.pdp.api.expression;
import org.ow2.authzforce.core.pdp.api.EvaluationContext;
import org.ow2.authzforce.core.pdp.api.IndeterminateEvaluationException;
import org.ow2.authzforce.core.pdp.api.StatusHelper;
import org.ow2.authzforce.core.pdp.api.StringUtils;
import org.ow2.authzforce.core.pdp.api.value.AttributeValue;
import org.ow2.authzforce.core.pdp.api.value.Datatype;
import org.ow2.authzforce.core.pdp.api.value.Value;
......@@ -61,7 +62,12 @@ public final class Expressions
}
final Value val = arg.evaluate(context);
LOGGER.debug("eval( arg = <{}>, <context>, expectedType = <{}> ) -> <{}>", arg, returnType, val);
if (LOGGER.isDebugEnabled())
{
LOGGER.debug("eval( arg = <{}>, <context>, expectedType = <{}> ) -> <{}>", StringUtils.sanitizeForLogging(arg), StringUtils.sanitizeForLogging(returnType),
StringUtils.sanitizeForLogging(val));
}
if (val == null)
{
throw NULL_ARG_EVAL_RESULT_INDETERMINATE_EXCEPTION;
......@@ -70,7 +76,8 @@ public final class Expressions
try
{
return returnType.cast(val);
} catch (final ClassCastException e)
}
catch (final ClassCastException e)
{
throw new IndeterminateEvaluationException("Invalid expression evaluation result type: " + arg.getReturnType() + ". Expected: " + returnType, StatusHelper.STATUS_PROCESSING_ERROR, e);
}
......@@ -90,7 +97,14 @@ public final class Expressions
public static AttributeValue evalPrimitive(final Expression<?> arg, final EvaluationContext context) throws IndeterminateEvaluationException
{
final Value val = arg.evaluate(context);
LOGGER.debug("evalPrimitive( arg = <{}>, <context>) -> <{}>", arg, val);
if (LOGGER.isDebugEnabled())
{
/*
* Findsecbugs: prevent CRLF log injection
*/
LOGGER.debug("evalPrimitive( arg = <{}>, <context>) -> <{}>", StringUtils.sanitizeForLogging(arg), StringUtils.sanitizeForLogging(val));
}
if (val == null)
{
throw NULL_ARG_EVAL_RESULT_INDETERMINATE_EXCEPTION;
......@@ -99,7 +113,8 @@ public final class Expressions
try
{
return (AttributeValue) val;
} catch (final ClassCastException e)
}
catch (final ClassCastException e)
{
throw new IndeterminateEvaluationException("Invalid expression evaluation result type: " + arg.getReturnType() + ". Expected: any primitive type", StatusHelper.STATUS_PROCESSING_ERROR, e);
}
......
......@@ -56,10 +56,13 @@ public final class DNSNameWithPortRangeValue extends SimpleValue<String>
private static final Pattern HOSTNAME_PATTERN;
static
{
final String domainlabel = "\\w[[\\w|\\-]*\\w]?";
final String toplabel = "[a-zA-Z][[\\w|\\-]*\\w]?";
/*
* Limit repetitions in regex to mitiate Regex DoS attacks
*/
final String domainlabel = "\\w[[\\w|\\-]{0,1000}\\w]?";
final String toplabel = "[a-zA-Z][[\\w|\\-]{0,1000}\\w]?";
// Add the possibility of wildcard in the left-most part (specific to XACML definition)
final String pattern = "[\\*\\.]?[" + domainlabel + "\\.]*" + toplabel + "\\.?";
final String pattern = "[\\*\\.]?[" + domainlabel + "\\.]{0,100}" + toplabel + "\\.?";
HOSTNAME_PATTERN = Pattern.compile(pattern);
}
......@@ -109,7 +112,8 @@ public final class DNSNameWithPortRangeValue extends SimpleValue<String>
// there is no port/portRange, so just use the name
host = dnsName;
range = NetworkPortRange.MAX;
} else
}
else
{
// split the name and the port/portRange
host = dnsName.substring(0, portSep);
......@@ -120,7 +124,7 @@ public final class DNSNameWithPortRangeValue extends SimpleValue<String>
// verify that the hostname is valid before we store it
if (!isValidHostName(host))
{
throw new IllegalArgumentException("Bad hostname: " + host);
throw new IllegalArgumentException("Bad hostname (invalid format, too many characters or too many levels): " + host);
}
return new SimpleEntry<>(host, range);
......
......@@ -220,9 +220,9 @@ public abstract class SimpleValue<V> extends AttributeValue
* @param datatypeId
* attribute datatype ID. MUST NOT be null.
* @param rawVal
* internal Java native value
* internal Java native value. MUST NOT be null.
* @throws java.lang.IllegalArgumentException
* if {@code datatype == null || jaxbVal == null}
* if {@code datatypeId == null || rawVal == null}
*/
protected SimpleValue(final String datatypeId, final V rawVal) throws IllegalArgumentException
{
......
......@@ -43,6 +43,52 @@ public final class X500NameValue extends SimpleValue<String>
private transient volatile int hashCode = 0; // Effective Java - Item 9
private static String escapeDN(final String name)
{
assert name != null;
final StringBuilder sb = new StringBuilder();
if (name.length() > 0 && (name.charAt(0) == ' ' || name.charAt(0) == '#'))
{
sb.append('\\'); // add the leading backslash if needed
}
for (int i = 0; i < name.length(); i++)
{
final char curChar = name.charAt(i);
switch (curChar)
{
case '\\':
sb.append("\\\\");
break;
case ',':
sb.append("\\,");
break;
case '+':
sb.append("\\+");
break;
case '"':
sb.append("\\\"");
break;
case '<':
sb.append("\\<");
break;
case '>':
sb.append("\\>");
break;
case ';':
sb.append("\\;");
break;
default:
sb.append(curChar);
}
}
if (name.length() > 1 && name.charAt(name.length() - 1) == ' ')
{
sb.insert(sb.length() - 1, '\\'); // add the trailing backslash if needed
}
return sb.toString();
}
/**
* Returns a new <code>X500NameAttributeValue</code> that represents the X500 Name value indicated by the string provided.
*
......@@ -56,8 +102,9 @@ public final class X500NameValue extends SimpleValue<String>
super(TYPE_URI, value);
try
{
this.ldapName = new LdapName(value);
} catch (final InvalidNameException e)
this.ldapName = new LdapName(escapeDN(value));
}
catch (final InvalidNameException e)
{
throw new IllegalArgumentException("Invalid value (X.500 Name) for datatype: " + TYPE_URI, e);
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment