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

Exploitable String Array Overflow Flaw #17

Open
HoratioGamer opened this issue Sep 10, 2023 · 4 comments
Open

Exploitable String Array Overflow Flaw #17

HoratioGamer opened this issue Sep 10, 2023 · 4 comments

Comments

@HoratioGamer
Copy link

HoratioGamer commented Sep 10, 2023

Ok, so Starscript can easily contain malicious scripts that are counter to the user's interest:

/whisper exploiter My Coords are {player.pos.x + "," + player.pos.z}

Such a script may appear as a macro in a Profile that an exploiter persuades a target to download and install. This is nothing new. The problem with this is, actually checking the macros before using them, I think most players would doubt that this macro is good for them. Is there a way to exploit the String Array Overflow Flaw to hide the exploiter's intentions. Yes.

It is described here: #16 (comment)

At present, I cannot think of a non-exploitable fix in the Compiler or Run procedures.

I will look to the Lexer or Parser to see if the script can be blocked at the input stage -- in effect most of the proposed script text would just turn red after the 256th string is reached in the text input box, and it would be impossible to save it as a script, so iit never gets compiled, and cannot be run.

@Paddyk45
Copy link

lmao what

@HoratioGamer
Copy link
Author

HoratioGamer commented Sep 10, 2023

The solution might be here: in Lexer.java lines 106-119...

    private void string() {
        while (!isAtEnd() && peek() != '"' && peek() != '\'') {
            if (peek() == '\n') line++;
            advance();
        }

        if (isAtEnd()) {
            createToken(Token.Error, "Unterminated expression.");
        }
        else {
            //
            //   Insert code here.... 
            //
            advance();
            createToken(Token.String, source.substring(start + 1, current - 1));
        }
    }

I am aware that unique strings are counted in a different class / different stage of compilation. The Lexer stage of compilation seems to be the only one capable of inserting the createToken(Token.Error, "Error message"); tokens, which I assume is how the input box in Meteor knows to color subsequent text red and show the error message. So if String Array Overflow is to be prevented, it has to be at this point. Code has to be entered at this point to build a list of unique string constants to count how many the compiler will later find. If there are too many unique ones, instead of createToken(Token.String, source.substring(start + 1, current - 1)); it has to createToken(Token.Error,"Error: too many strings"); The code already exists in class Script.java, writeConstant(). Not being familiar with Java, I would be tempted to just duplicate some code actions from Script in Lexer -- the string list in Lexer would not be used for anything but detecting if the Lexer has found more than 256 unique string constants.

@hellidox
Copy link

I will look to the Lexer or Parser to see if the script can be blocked at the input stage -- in effect most of the proposed script text would just turn red after the 256th string is reached in the text input box, and it would be impossible to save it as a script, so iit never gets compiled, and cannot be run.

ok but at that point you could just edit the nbt

@HoratioGamer
Copy link
Author

HoratioGamer commented Sep 14, 2023

As I see it there are 6 options:

OK, so I have never used an NBT editor before but I can see from a little reading I see it in the context of a world file in single player mode. I am going to guess what you are suggesting is editing a data structure Meteor saves the scripts to, perhaps where the profile is saved. Yes, one can hack the scripts on their own machine, bypassing the user input stage and the check by the Lexer. Yes, you are suggesting using the NBT editor at the exploiter's end to create a malicious profile that can be gifted to the mark. One would have to convince the mark to download and use a foreign profile. Coaching a mark through using an NBT editor on themselves was not what you were suggesting.

An additional mode of attack shut down by patching the Lexer as suggested would be, "here kid paste this script into a macro and activate it whenever you want to protect your secret base." The kid could read it, it would look legit, paste it in, it does something different than a plain reading, because of this vulnerability. That is something someone might be able to coach a mark through.

By pointing out the NBT option to get around my suggested fix, that is just saying either

  • the vulnerability itself has to be fixed more correctly, or,
  • that you do not care about there being a vulnerability and you are pointing out my fix is not comprehensive.

Yes, it would be better if StarScript were allowed to throw a runtime error, but, it has been made clear, it would affect too many projects if it did. So:

1) Fix it fully, by StarScript having the ability to throw runtime errors. (Eliminated by Dev)
2) Do nothing, because you don't care about scripts that execute differently than they are written.

Alternately, it could set a public Script.variable at run time that reflected if the script ran into any problems. Projects that care about code that executes differently than written could check the variable. Those that do not, don't need to bother. Meteor for instance may decide not to put anything in chat that is a runtime error. Or Meteor does nothing, because they do not care. No required changes to code, but, a notice to projects of the possibility of StarScript that does not execute as written. I have to believe in Java there is a few clever lines that say ignore exceptions thrown by this Forest.forest.smallforest -- in TCL it is just "catch { stringtochat = Forest.forest.smallforest } caught_error" -- using mixed languages. But no changes are allowed... so...

3) Create a Public variable at runtime that projects can check if they want to know if there has been a StarScript execution error. An optional error check available to projects.

Another alternative is, if the StarScript overflows the string constant space, it just returns the string it has been building in the stack. It is a different case of executing something different from a plain reading of the code, but, in general returning less string produces less problems. In the case of my exploit ternary above, it would do nothing, it would defeat my exploit at execution time. so...

4) Bail out without generating an error, just give results to the point of the error, refusing to execute the rest of the script that could only misbehave, and possibly be exploiting this vulnerability maliciously.

But what of a exploit written to exploit this bail out behaviour? Because there is no StarScript source code to POP the execution stack to remove data without using it, any exploit left on the line by a premature end of the script would have to lay out in code (that executes as it is written) a player exploitation. There are lots of player exploitations possible with no execution error in StarScript. The error or catching the error does not change that case.

Last option if the byte code for StarScript took a 2-byte string constant index, and allowed for 64k of string constants, again, my exploit would not work because 64k of string constants would overflow the Jump used in the outer ternary once compiled. Also, no kid could or would paste a 512k+ source string ("abcd" -- 7 source characters minimum in 64k of unique strings) into the input box in Meteor. Creating 64k of unique strings would be so arduous that it would be effectively unexploitable. At 256 strings, a believable script could be made just using the unique player names of 256 known stash raiders, or minecraft block designations -- anything that looks like game. A program that writes the script could generate 64k unique strings. At 64k unique strings, you would have to get into random strings of characters to make unique strings that would be rapidly unbelievable if the player glances at it. so....

5) Increase the string constant index space to 64k of string constants by having a 2-byte instead of a 1-byte index lookup.

6) Fix the Lexer as described even though it is not a comprehensive fix, because it is easy, and it would prevent the exploit of a mark by someone saying "here, paste this into a macro".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants