No Overflow Checking Array Index for Constants

[HoratioGamer]

starscript/src/main/java/org/meteordev/starscript

/Script.java lines 44-60

    public void writeConstant(Value constant) {
        int constantI = -1;

        for (int i = 0; i < constants.size(); i++) {
            if (constants.get(i).equals(constant)) {
                constantI = i;
                break;
            }
        }

        if (constantI == -1) {
            constantI = constants.size();
            constants.add(constant);
        }

        write(constantI);
    }

This depends on:

    private void write(int b) {
        if (size >= code.length) {
            byte[] newCode = new byte[code.length * 2];
            System.arraycopy(code, 0, newCode, 0, code.length);
            code = newCode;
        }

        code[size++] = (byte) b;
    }

That never checks if the value of b is greater than 127.

The declaration
public final List<Value> constants = new ArrayList<>();
does not set a limit on the size of the list as being 127.

So the following call cannot know whether we are exceeding the list size limit:
constants.add(constant);
Implicitly assumed by the particular application.

Only this cast of (byte):
code[size++] = (byte) b;
Might throw and error for values of b>127. Even if it does, the error cannot be descriptive of the true problem, that there are too many unique constants.

The limit of 127 unique constants would be a limit of StarScript for expansion more than the instruction pointer space.

In C there is (unsigned byte) which would double the number of a lot of things in StarScript... I do not know if that is a thing in JAVA.

I am wondering how Jump coding works with the ( >> 8) & 0xFF and & 0xFF if the data coded is & 0x7F as a rule because it is a Java byte.

It would seem no program could be over 127 bytes long and function properly ? Or if jumps are relative, which I am starting to wonder about because of the word offset in the code relating to jumps, then no ()?():() alternative block is allowed to code to more than 127 bytes -- which is less restrictive.

[HoratioGamer]

Yes, Jumps as written are relative not absolute addressing ip += jump

So the limit on script size is given by the type definition of int ip = 0;, so, 2 or 4 G of code.

[MineGame159]

Improved in f9d2946. The & 0xFF basically removes the sign bit when casting to a larger integer. I didnt add a proper error checking for the maximum number of constants because that would involve simulating the storage for constants in the parser. Also even if someone has a script with over 256 constants it will not throw an exception anymore, it will just roll back to zero.

[HoratioGamer]

Parser / Compiler.... I thought I was reading code from the latter, in which case the constants list is really being compiled, so, it could be a compiler error to have more than 255 constants?

And has a byte been redefined to be 0 to 255 from the Java standard of -128 to 127 ? Because the generic use of the = sign on assignment statements, and < > == in across comparisons generally recognizes the 8th bit as a sign unless the byte has been redefined.... and I am not yet sure about how Java defaults when it does evaluations... It could have been redefined, I cannot find it though.

[MineGame159]

It has not been redefined. As I already said, if you look at the commit I linked I just added & 0xFF where the constants list is accessed. And as I have already said that basically "gets rid" of the sign bit when the JVM converts the byte to an int. It will just kinda treat it as if the byte was unsigned.

[HoratioGamer]

        if (constantI == -1) {
            constantI = constants.size();
            constants.add(constant);
        }

becomes

        if (constantI == -1) {
           if (constant.size==128) {    // or 256 ?
              // Throw Exception with descriptive error message
           }
            constantI = constants.size();
            constants.add(constant);
        }

[MineGame159]

My point being was that everything that uses Starscript assumes that compilation will never produce an error. This means that code in Meteor, Craft Presence and probably other projects would have to be changed to handle this.

[HoratioGamer]

JVM converts the byte to an int

int to a byte maybe? byte = int & 0xFF ? Ok, I guess that works, if one never uses =,<,> signs with that byte and any data type that is not an int & 0xFF .... (int) 0x00000080 != (byte) 0x80

My point being was that everything that uses Starscript assumes that compilation will never produce an error. This means that code in Meteor, Craft Presence and probably other projects would have to be changed to handle this.

Ok, I see this... So it is the Parser that is giving the colour changes as you type the StarScript expressions detecting syntax errors that lead to parser errors. If the Parser sees no error, the assumption is the Compiler will see no error. Gotcha.

I know the limit is large for something that can be typed on one line, but that allows maliciously crafted strings pasted into the input box to do buffer overflows and change the action performed to be different from the plain reading of the line... I can't see a specific exploit at the moment... but that is saying nothing really.

[HoratioGamer]

The reason I cannot see an exploit is, the List remains true even if nothing over 256 can be accessed, it is what is executed after 256 strings that is not true. String 257 prints instead as the first string read on the line. Which repeats the first string the user sees much later on the line.

If the List wrapped around that would be bad because I could replace "/whisper friend My Coords are" at the beginning of a line that runs off the side of the text input box to actually execute "/whisper exploiter My Coords are" when it runs, by making the 257 string overwrite the first string.

[HoratioGamer]

Got a general strategy for an exploit of a String Array Overflow flaw in starscript to write a script that executes as a different script because of the flaw. The only reason to write a script that executes differently is to hide the script's intentions from someone who reads the script text. The only reason to do this is to create a script that will do something counter to a player's interests and trick them into running the script.

The entire Starscript formatting script below is a ternary expression that is false, to get the actually executed part of the script beyond the 256 string array limit -- that was the key in making the String Array Overflow flaw exploitable. The actual executed part of the script, because of the String Array Overflow flaw, actually executes as something different than what is written.

Ok, so, social engineering time.... the script ostensibly scans for a list of intruders near a base. Ostensibly means, on the surface, this is the fake-out, not its real, malicious action. It ostensibly sends a message to alert the player they are about to be under attack... This ability will sound really good to the mark -- the target of a scam. From there it is just salesmanship to get players to download a profile with this script in it, and for the mark to use it when they are at a super secret base. There are as many other possible social engineering things as there is imagination. Yes the brackets are probably the wrong types in the following schematic example.

(apparently obviously true but cleverly actually false) ? 
        ("Scanning for intruders" ... {
               ()?(innocent material including "/whisper " + "friend" + " We are about to be attacked at coordinates" +....  )
                  :(filler material with lots of strings to get the exploiters name as a string and create > 256 strings wrapping us back to exploiter's name-1)
           } ) 
 : 
 ("/whisper " + "fiend" + " We are about to be attacked at coordinates" ... )

Written as multiple lines to make it easier to see the blocks. Reading the compiler, scripts may have "\n" and multiple lines, which is useful for developing and pasting this exploit for testing on a 2nd private server.

The intentional misspelling of friend as fiend in its second appearance in the script, or, something even more not-obvious that will cause a string comparison failure will cause "fiend" to be created as a new string constant > 256 that is never actually used, but, in actual execution "exploiter" as the wrap-around string will be substituted.

Coordinates are sent to an exploiter. The social engineering may seem lame as a means to make the filler material seem useful. I guarantee riddler code that makes a kid's eyes cross before getting through it will work. IDC if it is not actually possible to check Notifier messages at present, the Lava Dupe never existed, but it still works as social engineering.

Have it as a Macro in a saved Profile, along with a package of other stuff -- more crap to audit, again to make a kid's eyes cross before finding anything definitely suspicious. Bind the malicious script to a commonly used key or the special P for protect my base ! Yes, the exploited target should be able to see the true message in chat, but by then it is too late -- they have given away the coordinates of the place they want to protect. An inattentive player may send their coordinates several times seeing the exploiter's name and maybe confusing it with someone who is there by Notifier. Adrenaline freezes thinking. Disbelief might be good for another button press.

Did I do good with my first (and second) ever use of a ternary ?

[HoratioGamer]

Thought of and eliminated a fix using constant.add("") so the first constant is an unused empty string, unless there is a string counter overflow, and then have all strings over 256 map to this first empty string. Unfortunately, it would only require a minor change to the exploit to still accomplish the "/whisper exploiter ..." to be delivered:

"/whisper " + "fiend" + " Watch Out " + "exploiter" + " at coordinates "

where "fiend" and " Watch Out " are >256th string, map to the empty string on execute, so it becomes

"/whisper exploiter at coordinates ".....

At run time.

Exploit remains.

[HoratioGamer]

I determined that I did not explain this well enough, edits above.

[HoratioGamer]

Oddly enough, this flaw was not exploitable when Jump instructions could not jump more than 256 characters of code.... #15 (comment) because the main ternary that allows the exploit would code to a Jump that would fail before the other edit.