How to prevent code injection with EL expression

Question

While I am using the JSF and I am trying to find the ValueExpression with below code:

public static final ValueExpression createValueExpression(
            final FacesContext context, final ELContext elContext,
            final String ev, final Class classType) {
        return context.getApplication().getExpressionFactory()
            .createValueExpression(elContext, ev, classType);
    }

But When I am running these code on HP fortify says that Interpreting user-controlled instructions at run-time can allow attackers to execute malicious code. It seems there is a risk of code injection with EL expression evaluation. But I know there is the code vulnerability so I want to know How Can I prevent the EL injection?

Could anyone help on the same?

score 2 · Accepted Answer · answered Mar 23 '18 at 06:17

Here expression string 'env' can be vulnerable to Expression Language Injection which occurs when attackers control data that is evaluated by an Expression Language interpreter.

For the solution, A more effective approach may be to perform data validation best practice against untrusted input and to ensure that output encoding is applied when data arrives on the EL layer, so that no metacharacter is found by the interpreter within the user content before evaluation. The most obvious patterns to detect include “${“ and “#{“, but it may be possible to encode or fragment this data.

So you can create a 'whitelist' pattern to match for the expression 'evn' before creating a value expression(whitelist can be something like: `[a-zA-Z0-9_.*#{}]*').

    Pattern pattern = Pattern.compile("[a-zA-Z0-9_\.\*#\{\}]*");
    Matcher matcher = pattern.matcher(ev);
    if (!matcher.matches()) {
        String message = "Detected a potential EL injection attack - 
              value["
            + ev+ "]"; 
         throw new Exception(message);
    }

Nice extensive answer but the user can still try to access any bean including beans he/she should not be allowed to. SO effectively there should be a whitelist of beans too. And more. I personally would redesign the application and wrap what the user can call in a single bean method and do checks there. — Kukeltje, Mar 23 '18 at 08:01

Kukeltje · Answer 2 · 2018-03-22T14:57:11.790

1

The risk is (just like with e.g. SQL injection) if you let the user provide input that is used in the EL (the ev String as correctly pointed out in the comments) without proper escaping and/or other basic security measures, you'll stand the risk that other code is executed than the one you want.

If this is not the case, you can ignore the warning (hope it is a warning, not a blocking error)

edited Mar 22 '18 at 14:57

answered Mar 22 '18 at 12:42

Kukeltje

12,223
4
24
47

yes the code is vulnerable to attack but I am not finding any solution for it, could you please help me out in finding the solution(I mean what security measures should I consider or if I can check for certain blacklist)? – Ashutosh Kumar Mar 22 '18 at 13:20
2

@AshutoshKumar the question is: is a user able to enter the `ev` string. If not, then you can suppress the warning. – Jasper de Vries Mar 22 '18 at 13:27
@JasperdeVries Here we can be supposed that the user can enter the ev string.So we need not wait for the attack. – Ashutosh Kumar Mar 23 '18 at 05:23

How to prevent code injection with EL expression

2 Answers2