Skip to content

Commit

Permalink
HV-1774 Test arbitrary code injection through buildConstraintViolatio…
Browse files Browse the repository at this point in the history
…nWithTemplate()
  • Loading branch information
yrodiere authored and gsmet committed May 6, 2020
1 parent 5903f44 commit df2fe2c
Showing 1 changed file with 51 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,20 @@ public void testAddParameterNodeForFieldLevelConstraintCausesException() throws
}
}

@Test
public void testInjectionCausedByRecklessConcatenation() {
String maliciousPayload = "$\\A{1 + 1}";

// Simulate user entry, through a web form for example
MyObjectWithELInjectionRiskCausedByRecklessConcatenation object = new MyObjectWithELInjectionRiskCausedByRecklessConcatenation();
object.field1 = maliciousPayload;
Set<ConstraintViolation<MyObjectWithELInjectionRiskCausedByRecklessConcatenation>> constraintViolations = validator.validate( object );
assertThat( constraintViolations ).containsOnlyViolations(
violationOf( ValidationWithELInjectionRiskCausedByRecklessConcatenation.class )
.withMessage( "Value '" + maliciousPayload + "' is invalid" )
);
}

@MyClassLevelValidation
private static class MyObject {
@NotNull
Expand Down Expand Up @@ -278,6 +292,13 @@ public String getName() {
}
}

@ValidationWithELInjectionRiskCausedByRecklessConcatenation
private static class MyObjectWithELInjectionRiskCausedByRecklessConcatenation {

String field1;

}

@Retention(RUNTIME)
@Constraint(validatedBy = MyClassLevelValidation.Validator.class)
public @interface MyClassLevelValidation {
Expand Down Expand Up @@ -486,4 +507,34 @@ public boolean isValid(String value, ConstraintValidatorContext context) {
}
}
}

@Retention(RUNTIME)
@Constraint(validatedBy = ValidationWithELInjectionRiskCausedByRecklessConcatenation.Validator.class)
public @interface ValidationWithELInjectionRiskCausedByRecklessConcatenation {
String message() default "failed";

Class<?>[] groups() default { };

Class<? extends Payload>[] payload() default { };

class Validator
implements ConstraintValidator<ValidationWithELInjectionRiskCausedByRecklessConcatenation, MyObjectWithELInjectionRiskCausedByRecklessConcatenation> {

@Override
public boolean isValid(MyObjectWithELInjectionRiskCausedByRecklessConcatenation value, ConstraintValidatorContext context) {
context.disableDefaultConstraintViolation();

// This is bad practice: message parameters should be used instead.
// Regardless, it can happen and should work as well as possible.
context.buildConstraintViolationWithTemplate( "Value '" + escape( value.field1 ) + "' is invalid" )
.addConstraintViolation();

return false;
}

private String escape(String value) {
return value.replaceAll( "\\$+\\{", "{" );
}
}
}
}

0 comments on commit df2fe2c

Please sign in to comment.