Skip to content

Commit

Permalink
Merge pull request #1007 from jpmorganchase/GSL_2020_034
Browse files Browse the repository at this point in the history
Close hibernate validator vulnerability
  • Loading branch information
Krish1979 authored Mar 4, 2020
2 parents 58fa9ba + 18f9673 commit b3d4699
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import javax.validation.ConstraintValidatorContext;
import java.nio.file.Path;
import java.util.Objects;
import java.util.regex.Pattern;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -37,7 +38,11 @@ public boolean isValid(Path t, ConstraintValidatorContext constraintContext) {
} catch (UncheckedIOException ex) {
LOGGER.debug(null, ex);
constraintContext.disableDefaultConstraintViolation();
constraintContext.buildConstraintViolationWithTemplate("Unable to create file " + t)

String sanitised = Objects.toString(t).replaceAll(Pattern.quote("$"),"")
.replaceAll(Pattern.quote("#"),"");
String message = String.format("Unable to create file %s",sanitised);
constraintContext.buildConstraintViolationWithTemplate(message)
.addConstraintViolation();
return false;
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -186,4 +188,59 @@ public void checkCannotCreateFileDontCheck() {

verifyNoMoreInteractions(context);
}


@Test
public void steathElExpression() throws Exception {

final String evilPathWithElExpression = "/somepath/${somethingbad}/somefile.file";

ValidPath validPath = mock(ValidPath.class);
when(validPath.checkCanCreate()).thenReturn(true);
when(validPath.checkExists()).thenReturn(false);
PathValidator pathValidator = new PathValidator();
pathValidator.initialize(validPath);

Path path = Paths.get(evilPathWithElExpression);

ConstraintValidatorContext context = mock(ConstraintValidatorContext.class);

List<String> messages = new ArrayList<>();
doAnswer(invocation -> {
messages.add(invocation.getArgument(0));
return mock(ConstraintValidatorContext.ConstraintViolationBuilder.class);
}).when(context).buildConstraintViolationWithTemplate(anyString());

assertThat(pathValidator.isValid(path,context)).isFalse();

assertThat(messages).containsExactly("Unable to create file /somepath/{somethingbad}/somefile.file");

}

@Test
public void steathElExpressionHashPrefix() throws Exception {

final String evilPathWithElExpression = "/somepath/#{somethingbad}/somefile.file";

ValidPath validPath = mock(ValidPath.class);
when(validPath.checkCanCreate()).thenReturn(true);
when(validPath.checkExists()).thenReturn(false);
PathValidator pathValidator = new PathValidator();
pathValidator.initialize(validPath);

Path path = Paths.get(evilPathWithElExpression);

ConstraintValidatorContext context = mock(ConstraintValidatorContext.class);

List<String> messages = new ArrayList<>();
doAnswer(invocation -> {
messages.add(invocation.getArgument(0));
return mock(ConstraintValidatorContext.ConstraintViolationBuilder.class);
}).when(context).buildConstraintViolationWithTemplate(anyString());

assertThat(pathValidator.isValid(path,context)).isFalse();

assertThat(messages).containsExactly("Unable to create file /somepath/{somethingbad}/somefile.file");

}
}

0 comments on commit b3d4699

Please sign in to comment.