Changes after review 1

Changed the base64 pattern to only accept token68 pattern from rfc7235#appendix-C
Add limit to recusion depth of multiple challange matching to stop any vulnerablilties related to malicious server overflowing client stack
Regex no longer allows trailing whitespace

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2018-04-24 11:16:31 +10:00
parent 5a1325949e
commit fc5c438d72
2 changed files with 16 additions and 7 deletions

View File

@ -49,8 +49,9 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler
private static final Pattern PARAM_PATTERN = Pattern.compile("([^=]+)=([^=]+)?"); private static final Pattern PARAM_PATTERN = Pattern.compile("([^=]+)=([^=]+)?");
private static final Pattern TYPE_PATTERN = Pattern.compile("([^\\s]+)(\\s+(.*))?"); private static final Pattern TYPE_PATTERN = Pattern.compile("([^\\s]+)(\\s+(.*))?");
private static final Pattern MULTIPLE_CHALLENGE_PATTERN = Pattern.compile("(.*),\\s*([^=\\s,]+(\\s+[^=\\s].*)?)\\s*"); private static final Pattern MULTIPLE_CHALLENGE_PATTERN = Pattern.compile("(.*),\\s*([^=\\s,]+(\\s+[^=\\s].*)?)");
private static final Pattern BASE64_PATTERN = Pattern.compile("([^\\s,=]+=*)\\s*"); private static final Pattern BASE64_PATTERN = Pattern.compile("[\\+\\-\\.\\/\\dA-Z_a-z~]+=*");
private static final int MAX_DEPTH = 10;
private final HttpClient client; private final HttpClient client;
private final int maxContentLength; private final int maxContentLength;
@ -85,12 +86,20 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler
protected List<HeaderInfo> getHeaderInfo(String value) throws IllegalArgumentException protected List<HeaderInfo> getHeaderInfo(String value) throws IllegalArgumentException
{ {
return getHeaderInfo(value, 0);
}
protected List<HeaderInfo> getHeaderInfo(String value, int depth) throws IllegalArgumentException
{
if(depth > MAX_DEPTH)
throw new IllegalStateException("too many challanges");
Matcher m = MULTIPLE_CHALLENGE_PATTERN.matcher(value); Matcher m = MULTIPLE_CHALLENGE_PATTERN.matcher(value);
if (m.matches()) if (m.matches())
{ {
List<HeaderInfo> l = new ArrayList<>(); List<HeaderInfo> l = new ArrayList<>();
l.addAll(getHeaderInfo(m.group(1))); l.addAll(getHeaderInfo(m.group(1), depth+1));
l.addAll(getHeaderInfo(m.group(2))); l.addAll(getHeaderInfo(m.group(2), depth+1));
return l; return l;
} }
else else
@ -128,7 +137,7 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler
Matcher b64 = BASE64_PATTERN.matcher(wwwAuthenticate); Matcher b64 = BASE64_PATTERN.matcher(wwwAuthenticate);
if (b64.matches()) if (b64.matches())
{ {
result.put("base64", b64.group(1)); result.put("base64", wwwAuthenticate);
return result; return result;
} }
@ -139,7 +148,7 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler
if (params.matches()) if (params.matches())
{ {
String name = StringUtil.asciiToLowerCase(params.group(1)); String name = StringUtil.asciiToLowerCase(params.group(1));
String value = (params.group(2)==null)?"":params.group(2); String value = (params.group(2)==null) ? "" : params.group(2);
result.put(name, value); result.put(name, value);
} }
else else

View File

@ -689,7 +689,7 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
Assert.assertTrue(headerInfo.getType().equalsIgnoreCase("Scheme")); Assert.assertTrue(headerInfo.getType().equalsIgnoreCase("Scheme"));
Assert.assertTrue(headerInfo.getParameter("realm") == null); Assert.assertTrue(headerInfo.getParameter("realm") == null);
List<HeaderInfo> headerInfos = aph.getHeaderInfo("Scheme1 , Scheme2 "); List<HeaderInfo> headerInfos = aph.getHeaderInfo("Scheme1 , Scheme2");
Assert.assertEquals(headerInfos.size(), 2); Assert.assertEquals(headerInfos.size(), 2);
Assert.assertTrue(headerInfos.get(0).getType().equalsIgnoreCase("Scheme1")); Assert.assertTrue(headerInfos.get(0).getType().equalsIgnoreCase("Scheme1"));
Assert.assertTrue(headerInfos.get(1).getType().equalsIgnoreCase("Scheme2")); Assert.assertTrue(headerInfos.get(1).getType().equalsIgnoreCase("Scheme2"));