Il2Cpp.String::content may write out of bounds

[axhlzy]

https://github.com/vfsfitvnm/frida-il2cpp-bridge/blob/e250654cff37a8213ee03f19baa2cda131991609/src/il2cpp/structs/string.ts#L16

It seems that you should not directly write the memory in this way. You have modified the length of the string here, and the content has also been extended, which will cause the memory of the latter part to be overwritten.

[vfsfitvnm]

Yeah, correct, thanks for noticing. I should allocate a new string in case its length increases.

However, there's another place in the code base where it might end up writing more bytes than it should: can you spot it? :stuck_out_tongue_closed_eyes:

[axhlzy]

not yet ... but It seems that this function can be used to modify the length of the original string

il2cpp_string_new_len

[vfsfitvnm]

il2cpp_string_new_len is invoked by il2cpp_string_new: https://github.com/dreamanlan/il2cpp_ref/blob/09316fe508773b8ced098dae6147b44ee1f6516c/libil2cpp/vm/String.cpp#L72

[vfsfitvnm]

Uhm, it looks like the Il2CppString struct should stay immutable:

typedef struct Il2CppString
{
    Il2CppObject object;
    int32_t length;
    Il2CppChar chars[0];
} Il2CppString;

Il2CppString* String::NewSize(int32_t len)
{
    if (len == 0)
        return Empty();

    Il2CppString *s;
    IL2CPP_ASSERT(len >= 0);
    size_t size = (sizeof(Il2CppString) + ((len + 1) * 2));

    s = reinterpret_cast<Il2CppString*>(Object::AllocatePtrFree(size, il2cpp_defaults.string_class));

    s->length = len;
    s->chars[len] = 0;

    return s;
}

In other words, we can't grow the string without reallocating the whole memory, and Il2Cpp.String::handle is immutable as well. Now, we all agree the following snippet:

const s = Il2Cpp.string("hello");
s.content = "a longer content";

ends up writing to unallocated memory, potentially causing debug nightmares, and it should be prohibited.

However, writing a shorter string should be completely fine, so we could either: 1) Make Il2Cpp.String::content read only; 2) Keep Il2Cpp.String::content as it is and: 1) Warn the user if a write out of bounds is about to occur; 2) Add Il2Cpp.String::contentUnsafe for those who feel lucky.

[axhlzy]

Il2cppString is actually equivalent to this class (mscorlib System.String), so you can directly use the methods under this class to modify String

SUB -> mscorlib.System.String

System.String.Concat(cs_string* left, cs_string* right)
System.String.CreateString(char* array, int start, int length)
System.String.FastAllocateString(int length)
System.String.Substring(cs_string* this, int start, int length)
System.String.Replace(cs_string* original, cs_string* old, cs_string* new)

[vfsfitvnm]

These methods return a new string!

[vfsfitvnm]

I eventually decided to leave things as they are (I simply added an @unsafe marker to the setter docstring), as one might not care at all of this discouraged pattern.

However, I will take care of updating the documentation so that it's clear overwriting the string content may write out of bounds.