Mtasa-blue: Tea encoding bug

[ZbyK77]

Describe the bug

My version: Multi Theft Auto v1.5.7-release-20520
The problem occurs when using the encodeString, decodeString, teaDecode and teaEncode functions. The characters "\0" are added to the decrypted string. Exists only on the client side, and when the number of characters is not a multiple of 4.

To reproduce
Use the iprint(decodeString("tea",encodeString("tea",1,{key="123"}),{key="123"}))

Expected behaviour
Return the correct value

Screenshots

none

Version

Client Windows
Multi Theft Auto v1.5.7-release-20520
Copyright (C) 2003 - 2020 Multi Theft Auto

Additional context

Example:
iprint(decodeString("tea",encodeString("tea",1,{key="123"}),{key="123"}))
returns "1\0\0\0" instead of "1"

More examples:
String before encoding - String after decoding

"1" - "1\0\0\0"
"12" - "12\0\0"
"123" - 123\0"
"1234" - "1234"
"1234567" - "1234567\0"
"12345678" - "12345678"

[botder]

Looks like padding and could be removed

[ZbyK77]

Of course, this isnt a solution to the problem.
However, I use this to encode saved passwords in client files. Removing "\0" from the string would cause errors with password matching.

I noticed that the problem also occurs on the server side (MTA: SA Server v1.5.7-release-20423) but there only when we use the function decodeString / encodeString, it doesnt occur when using the function teaEncode / teaDecode.

> iprint(teaDecode(teaEncode(1,"123"),"123")
return c-side: "1\0\0\0"
return s-side: "1"

> iprint(decodeString("tea",encodeString("tea",1,{key="123"}),{key="123"}))
return c-side: "1\0\0\0"
return s-side: "1\0\0\0"

Returning to the client, on previous versions it works correctly, among others on Multi Theft Auto v1.5.7-release-20447.2.

[Pirulax]

Its pretty bad if you use this to save passwords.
If its for some kind of login panel, i suggest using session tokens instead.

I was about to comment something, but relaized your version is much older(20423) than the version i thought introduced it.

try going down to some other revision, see where the bug has been introduced.

[Pirulax]

I think i've found it.
Its buried deep under the implementation, but:

// pushed in CLuaArgument::Push(), used in en/decodeString
lua_pushlstring(luaVM, m_strString.c_str(), m_strString.length());

// pushed in the new parser(Used in teaEn/Decode
lua_pushlstring(L, value.data(), value.length());

Im not totally sure why would it add that padding of null terminators there, i mean why it includes it.

[ccw808]

I think the padding has always been there, but the old CLuaCryptDefs::TeaDecode function stripped it by using lua_pushstring(luaVM, str)

I guess the solution would be to return strOutResult.c_str() from the new CLuaCryptDefs::TeaDecode

[Pirulax]

This strips it:

    const auto end = str.end();

    auto iter = end;
    while (*(--iter) == 0);

    ++iter;

    if (iter != end)
        str.erase(iter, end);

with str being the encoded value.

[qaisjp]

This also works:

std::string_view s = "hello\0\0\0\0"s;

for (auto it = s.end() - 1; it != s.begin(); it--) {
  if (*it == 0)
    continue;
  s = s.substr(0, s.end() - it);
  break;
}

return s;

[Pirulax]

We unfortunately cant just return the string view, we must modify the string itself, so we can return that.

[Pirulax]

Expected behaviour
Return the correct value

And what are the correct values?????

[ccw808]

As this is a regression, it should be fixed ASAP. What is wrong with returning strOutResult.c_str() i.e. a new std::string with zeros trimmed?

[Pirulax]

It's okay, I'm working on it, quite an easy fix.

[Pirulax]

@ZbyK77 Could you please post the expected values?

[qaisjp]

In the examples on the bottom of the issue, the left is expected, right is actual

[Pirulax]

Well, I'm pretty sure it strips all zeros, no matter if the user added them or not, because strlen stops at the first 0 found.