We have a spring-boot (2.4.2) application and an Aspect class that does some handling "Around" methods annotated with a custom annotation, defined by us, and using SpEL handling.
The SpEL expressions are defined as fields in the annotation, by us.
When running the Sonar tool, together with Findsecbugs, we are told that we have a vulnerability in the code, with the error "This use of org/springframework/expression/ExpressionParser.parseExpression(Ljava/lang/String;)Lorg/springframework/expression/Expression; could be vulnerable to code injection (Spring Expression)". The offending line is line 4 below:
1. private final ExpressionParser elParser = new SpelExpressionParser();
...
2. @Around(value = "@annotation(myCustomAnnotation)")
3. public Object aroundAdviceHandler(ProceedingJoinPoint joinPoint, MyCustomAnnotation myCustomAnnotation) throws Throwable {
...
4. **Expression expression = elParser.parseExpression(myCustomAnnotation.entityId());**
The annotated code that uses this Aspect looks like:
@Transactional
@MyCustomAnnotation(entityId = "[0].id") // some other methods my have here only "[0]" or "[0].otherId"
public Long save(JustADto dto) {
And finally, the custom Annotation looks like:
@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME) public
@interface MyCustomAnnotation {
String entityId() default "";
}
This code doesn't seem to have any vulnerability, because the input for the spring expression is provided by us. Is this a false positive by Findsecbugs? Is there any way to prevent the Sonar & Findsecbugs error from appearing, other than using the <@SuppressFBWarnings(value = {"SPEL_INJECTION"}, justification = "false positive")> annotation?
The value returned by MyCustomAnnotation.entityId() is an expression that is hard-coded in your code. Because of this I would consider the source to be safe.
You can trust the developers who wrote the code. You can't trust remote users. Here the value is only controlled by the developer.
Side note: I think it would be reasonable that the tool considered annotations as always safe. I created a ticket to this effect.