Some OOM problem was found in rhino

[Alex111998]

description

When I test the latest version(1.7.14) of rhino by CIFuzz，OOM security issue was found when use the follow apis,may cause denial of service issues in applications when use unlimited:

pom

<dependency>
        <groupId>org.mozilla</groupId>
        <artifactId>rhino</artifactId>
        <version>1.7.14</version>
</dependency>

code

import org.mozilla.javascript.Kit;
import org.mozilla.javascript.ast.DoLoop;
import org.mozilla.javascript.ast.ElementGet;
import org.mozilla.javascript.ast.EmptyExpression;
import org.mozilla.javascript.ast.EmptyStatement;
import org.mozilla.javascript.ast.ForInLoop;
import org.mozilla.javascript.ast.ForLoop;
import org.mozilla.javascript.ast.FunctionCall;
import org.mozilla.javascript.ast.FunctionNode;
import org.mozilla.javascript.ast.GeneratorExpressionLoop;
import org.mozilla.javascript.ast.IfStatement;
import org.mozilla.javascript.ast.InfixExpression;
import org.mozilla.javascript.ast.KeywordLiteral;
import org.mozilla.javascript.ast.Label;
import org.mozilla.javascript.ast.LetNode;
import org.mozilla.javascript.ast.Name;
import org.mozilla.javascript.ast.NewExpression;

public class Rhino_OOM {

    public static void main(String[] args) {
        
        // api 1
        try {
            DoLoop doLoop = new DoLoop();
            String result = doLoop.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 2
        try {
            ElementGet elementGet = new ElementGet();
            String result = elementGet.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 3
        try {
            EmptyExpression emptyExpression = new EmptyExpression();
            String result = emptyExpression.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 4
        try {
            EmptyStatement emptyStatement = new EmptyStatement();
            String result = emptyStatement.toSource(1093157160);
        } catch (Exception e) {
        }

        // api 5
        try {
            ForInLoop forInLoop = new ForInLoop();
            String result = forInLoop.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 6
        try {
            ForLoop forLoop = new ForLoop();
            String result = forLoop.toSource(1594505994);
        } catch (Exception e) {
        }

        // api 7
        try {
            FunctionCall functionCall = new FunctionCall();
            String result = functionCall.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 8
        try {
            FunctionNode functionNode = new FunctionNode();
            String result = functionNode.toSource(168430173);
        } catch (Exception e) {
        }

        // api 9
        try {
            GeneratorExpressionLoop generatorExpressionLoop = new GeneratorExpressionLoop();
            String result = generatorExpressionLoop.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 10
        try {
            IfStatement ifStatement = new IfStatement();
            String result = ifStatement.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 11
        try {
            InfixExpression infixExpression = new InfixExpression();
            String result = infixExpression.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 12
        try {
            KeywordLiteral keywordLiteral = new KeywordLiteral();
            String result = keywordLiteral.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 13
        try {
            byte[] result = Kit.readStream(null, Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 14
        try {
            Label label = new Label();
            String result = label.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 15
        try {
            LetNode letNode = new LetNode();
            String result = letNode.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 16
        try {
            Name name = new Name();
            String result = name.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }

        // api 17
        try {
            NewExpression newExpression = new NewExpression();
            String result = newExpression.toSource(Integer.MAX_VALUE);
        } catch (Exception e) {
        }
    }
}

analysis

All of these apis are Unlimited concatenation strings, a large amount of memory is consumed and may occurs OOM.

[tuchida]

Is this the problem that Rhino should fix?
For example, the following code results in an infinite loop, which is the problem for the user or Rhino?

import org.mozilla.javascript.ast.Block;

public class Rhino_OOM {
    public static void main(String[] args) {
        Block block = new Block();
        block.addChild(block);
        System.out.println(block.toSource());
    }
}

[rbri]

@tuchida agree, but maybe in the special case of indention we can protect again this and do a minor improvement be pre calculating the values to avoid the creation of the indent string over and over again.
see my pr #1428 - feedback welcome

[tuchida]

There are several ways to optimize string repetition. ref. #188
Standard methods have been added in Java 11.
I don't see much benefit in speeding this up now.

[tuchida]

OOM security issue

Why is this a security issue? DoS?
For example, would throwing IllegalArgumentException instead, solve the problem?

---

This is my opinion, but it is not a security issue that Rhino users can trigger OOM. If there is a service created by a Rhino user, and the service user of that service can trigger OOM in a way that is not intended by the Rhino user, could be a security issue in Rhino.