the following code: // check for tag inside itself eg &

Do you need to check for contained tags in riot's chunked xmpp messages? about crossplatformplaying HOT 4 CLOSED

giorgi-o commented on June 11, 2024

Do you need to check for contained tags in riot's chunked xmpp messages?

from crossplatformplaying.

Comments (4)

giorgi-o commented on June 11, 2024

When you send a DM (whisper) to someone, this is what is sent through XMPP:

<message from='[puuid]@eu1.pvp.net/RC-3761796501' to='[email protected]' type='chat' xmlns='jabber:client'>
  <sent xmlns='urn:xmpp:carbons:2'>
    <forwarded xmlns='urn:xmpp:forward:0'>
      <message to='[puuid]@eu1.pvp.net' stamp='2022-03-01 13:08:19.721' id='1646140099775:2' type='chat'>
        <body>hello</body>
      </message>
    </forwarded>
  </sent>
</message>

As you can see, there are two <message> tags, one contained within the other. This causes problems because I'm too lazy to write a proper XML parser :P, so the plugin just looks for the next </message> and assumes that's the one that matches the first one. It would then see that there is some more text after the "last" tag and assumes it's a split message (riot splits large messages) and waits for the rest, which never comes, hence it just stops processing incoming messages, and even causes a memory leak since it stores all incoming messages hoping to one day process them. All because of a DM.

So I wrote a quick piece of code to handle nested tags with the same name and so far it seems to be working. Since I don't do anything with DMs it doesn't have to be ultra robust, it just has to prevent it from hanging.

it seems that riot encodes '<' '>' inside a message as '&let;' & so on

I actually didn't know that! I guess presence injection isn't a thing after all :)

from crossplatformplaying.

M-U-X commented on June 11, 2024

Thank you for replying, I didn't know that regarding the message xml tag & didn't notice it, & yes, I think they changed it a while ago to prevent using things like
Red: Text</>
Blue: Text</>
Yellow: Text</> or Text</>
Green: Text</>
Pink: Text</> or Text</> to change chat color in valorant IIRC, at least when I tried to do a presence injection it didn't work and inspecting what's sent it was like I described earlier, but I think I actually found a bug in the processing code, I've no idea if it'll behave the same in javascript or if it's "not a bug" in javascript, here's the code @ :
`if(!data.startsWith('<')) return err("RIOT: xml presence data doesn't start with '<'! " + data);

                    const firstTagName = data.substring(1, data.indexOf('>')).split(' ', 1)[0];`

say you received a split message that goes something like this:
<presence>........</presence><presence>......</presence><presence from='[puuid]@[].pvp.net' to='20864Ae2-b
the processing loop will take the first complete presence then sets bufferedMessage to bufferedMessage = data.substr(firstTagEnd); // will be empty string if only one tag @Line1568 process the presence data, sets data to "", then go to the second one which is now first in the buffered message, do the same resulting in bufferedMessage="<presence from='[puuid]@[].pvp.net' to='20864Ae2-b", and starts the third loop, it'll pass this check if(!data.startsWith('<')) return err("RIOT: xml presence data doesn't start with '<'! " + data); because it does start with a '<' then it'll execute const firstTagName = data.substring(1, data.indexOf('>')).split(' ', 1)[0]; but data.indexOf('>') will return -1 & you aren't checking for that in the code, solution
let firstTagEndingIndex = data.indexOf('>'); if (firstTagEndingIndex === -1){ // message is split, we need to wait for the end bufferedMessage = data; break; }

from crossplatformplaying.

giorgi-o commented on June 11, 2024

I didn't consider the case where the message could be chunked in the middle of a tag! Well spotted.

That being said, the code will actually still function as intended, and here's why:

Say that the message received is <presence from='[puuid]@[].pvp.net' to='20864Ae2-b. As you point out, the line that this should fail on is:

const firstTagName = data.substring(1, data.indexOf('>')).split(' ', 1)[0];

because data.indexOf('>') will be -1, so in the end, the code that will be run is:

let s = "<presence from='[puuid]@[].pvp.net' to='20864Ae2-b";
const firstTagName = s.substring(1, -1);

Which, in any other language, would either fail with an error, or return an empty string. After all, getting the substring from 1 to -1 doesn't really make sense, does it?

But, javascript is javascript, and as the MDN docs tell us for substring():

If indexStart is greater than indexEnd, then the effect of substring() is as if the two arguments were swapped

So, the code that is actually run is:

let s = "<presence from='[puuid]@[].pvp.net' to='20864Ae2-b";
const firstTagName = s.substring(-1, 1); // '<'

Which seems like it should lead to a crash at some point, right? Well, not exactly. What the code then does is:

let closingTagIndex = data.indexOf(`</${firstTagName}>`); // data.indexOf("</<>");
if(closingTagIndex === -1) { // true
    // message is split, we need to wait for the end
    bufferedMessage = data;
    break;
}

So we can see that despite everything, the code correctly identifies the message as being chunked. Thanks JS!

(That being said, this is of course a lucky hack, and better safe then sorry. I'll fix it in the next update)

from crossplatformplaying.

M-U-X commented on June 11, 2024

I'm working in c++ which of course caused problems but I had a suspicion that maybe it doesn't here due to JavaScript weirdness, thank you for the detailed explanation though, JavaScript is fascinating to say the least :)

from crossplatformplaying.

Do you need to check for contained tags in riot's chunked xmpp messages? about crossplatformplaying HOT 4 CLOSED

Comments (4)

Related Issues (9)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent