h3x4n1um / RETON

Reverse Engineering RTON File
GNU General Public License v3.0
33 stars 3 forks source link

Boundary Check&parsing problem #3

Closed FinBird closed 5 years ago

FinBird commented 5 years ago

I carefully checked rton2json.cpp and found some problems: 1.Check if cache string exist in vector Creat a string stack,that's ok:

    std::vector <std::string> stack_0x91;
    std::vector <std::string> stack_0x93;

But need to handle error...

     case 0x91:{
            //recall
            res.push_back(stack_0x91[unsigned_RTON_num2int(read_RTON_num())]);  //Naive, you don't know if a cache string in the vector now
     }
    // ...
    case 0x93:{
            //recall
            res.push_back(stack_0x93[unsigned_RTON_num2int(read_RTON_num())]);  //As above...
            break;
        }

Read the memory out of len may cause undefine behavior! To solve this ,a possible solution can be: 1.Creat a class handle it.(troublesome,but rubust) 2.Just set a variable to keep the number of cache string.

  1. Braces of json are in pairs:
         //end of object
        case 0xFF:{  //check count 0x85 = 0xff ???,should not check it until the parser panic...
            break;
        }

    The 0x85 and 0xff should be handle as a pair.(Treat it as the braces in caculator) What if someone writes 0x85 0x90 0x01 0x6d 0x24 0x01 0xff 0xff in RTON... !!! 3.Key-Value pair problem: Basically everything in json is key:value pairs and values

        "objclass": "TideProperties", "aliases": [ "Tide" ] 

    And the Root of a json must be a map(0x85):

    {
    "#comment": "Embedded RSB Version",
    "version": 1,
    "objects": [
        {
            "objclass": "PVZVersion",
            "objdata": {
                "major": 0,
                "minor": 0,
                "content": 0
            }
        }
    ]
    }

    Then,the key only can be String(0x90/91/81)and values can be any of them (0x83/92/93/24/85/86) Array(0x86) only contains values

"Plants":["Peanut","Sunflower",{"A":"B"}]


So,you need to edit the logic of Parsing,using recursive descent parsing or whatever...
h3x4n1um commented 5 years ago

I have 2 final exams in the last 2 weeks of June, I'll look to it in July thanks

LambdaEd1th commented 5 years ago

To give some ideas, I have made some references which is from my C# project and try to translate into C++.

json::value_t ValueMethod(ifstream& ifs)
{
    json Value;
    uint8_t ReadBuffer;
    ifs.read((char*)ReadBuffer, sizeof(ReadBuffer));
    switch (ReadBuffer)
    {
    /*
        Detail values codes should be here.
    */
    case 0x86:
    {
        int ArraySize;  //7 Bit Encoded Int
        ifs.read((char*)ReadBuffer, sizeof(ReadBuffer));
        if (ReadBuffer == 0xfd)
        {
            json::array_t ArrayValue;
            for (int i = 0; i < ArraySize; i++)
            {
                ArrayValue.push_back(ValueMethod(ifs));
            }
            ifs.read((char*)ReadBuffer, sizeof(ReadBuffer));
            if (ReadBuffer == 0xfe)
            {
                Value.push_back(ArrayValue);
            }
        }
        break;
    }
    case 0x85:
    {
        json::object_t ObjectValue = ObjectMethod(ifs);
        Value = ObjectValue;
        break;
    }
    }

    return Value;
}

json::object_t ObjectMethod(ifstream& ifs)
{
    json::object_t ObjectValue;
    uint8_t ReadBuffer;
    for (ifs.read((char*)ReadBuffer, sizeof(ReadBuffer)); ReadBuffer != 0xff; ifs.read((char*)ReadBuffer, sizeof(ReadBuffer)))
    {
        string StringValue; //key
        switch (ReadBuffer)
        {
        case 0x90:
        {
            //...
        }
        case 0x91:
        {
            //...
        }
        }
        json::value_t Value = ValueMethod(ifs);

        /*
            key-value pairs codes should be here.
        */
    }

    return ObjectValue;
}
h3x4n1um commented 5 years ago

Key-value problem

As we know, JSON has type <key, value> in general and RTON is basically PopCap binary version of JSON

In JSON, it only has those types:

And both key and value can only be 1 of those types (except key can only be string) so I called it a block to make it easy to write a recursive function (write_RTON_block and read_RTON_block)

The code that use to decode or encode is actually read_RTON and write_RTON

json read_RTON(){
    //decoding
    json res;
    while(true){
        std::string key;
        json js_key = read_RTON_block();
        if (js_key.size() == 0) return res;
        else{
            if (!js_key[0].is_string()){
                std::cerr << std::endl << "ERROR! KEY IS NOT A STRING!!!" << std::endl;
                debug << std::setw(4) << debug_js;
                std::cin.get();
                exit(1);
            }
            key = js_key[0];
        }
        //prevent push entire array lol
        json value = read_RTON_block()[0];
        res[key] = value;
    }
}
int write_RTON(json js){
    for (auto i : js.get<std::map <std::string, json> >()){
        write_RTON_block(i.first);
        write_RTON_block(i.second);
    }
    debug_js["RTON Stats"]["List of Bytecodes"][to_hex_string(output.tellp())] = to_hex_string(object_end);
    output.write(reinterpret_cast<const char*> (&object_end), 1);
    return 0;
}

Which is validated the key requirement is can only be string (I'm not sure if 0x92, 0x93 and 0x82 are allowed)

Bracket in RTON

IIRC, this is also a valid RTON

52 54 4F 4E 01 00 00 00
FF
44 4F 4E 45

Base on the example, we can see RTON bracket are not equal number of open brackets + 1 = number of close brackets

If someone writes this in RTON

52 54 4F 4E 01 00 00 00
FF FF FF
44 4F 4E 45

The parser will accept it as a valid RTON although it is not correct in RTON format

The solution here is we just need to add a variable bracket, when using 0x85 we increase it to one and using 0xff we decrease it to one, if bracket < -1 then just put error

Stack overflow (no punchline intended XD)

I guess I can resolve this by checking does element i-th is currently in the stack?

h3x4n1um commented 5 years ago

Fixed in rton-json v2.7.2

FinBird commented 5 years ago

Good job!I think you have resolved this by paying those effort! @h3x4n1um