skynet-im / skynet-android

Skynet Messenger Android client
GNU General Public License v3.0
1 stars 0 forks source link

Random byte packet contents #9

Closed daniel-lerch closed 4 years ago

daniel-lerch commented 5 years ago

Describe the bug Sometimes some clients get kicked by the server because of sending random packet contents that violate protocol rules.

Expected behavior The client should not send packets that violate protocol rules.

Server logs

Starting to handle packet {P34SetClientState}
Successfully sent packet {P2CChannelAction}
Successfully sent packet {P2CChannelAction}
Successfully sent packet {P2CChannelAction}
Starting to handle packet {P0ACreateChannel: ChannelId=317c1c267721da01 Type=63 Owner=c3024e28507a8a78 Counterpart=00000000}
Connection closed: A System.ArgumentOutOfRangeException was thrown in user code as has not been handled System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values.
   at SkynetServer.Network.Client.Handle(P0ACreateChannel packet) in /app/src/SkynetServer/Network/Client.PacketHandler.cs:line 315
   at SkynetServer.Network.Client.OnPacketReceived(Byte id, Byte[] content) in /app/src/SkynetServer/Network/Client.cs:line 88
   at VSL.VSLSocket.OnPacketReceived(Byte id, Byte[] content)
Successfully sent packet {P0BChannelMessage: ContentId=2b ChannelId=caf922b466760441 MessageId=000000cd Flags=Unencrypted, NoSenderSync}
Starting to handle packet {P34SetClientState}
Successfully sent packet {P2CChannelAction}
Successfully sent packet {P2CChannelAction}
Starting to handle packet {P34SetClientState}
Successfully sent packet {P2CChannelAction}
Starting to handle packet {P34SetClientState}
Successfully sent packet {P2CChannelAction}
Connection closed: A SkynetServer.Network.ProtocolException was thrown in user code as has not been handled SkynetServer.Network.ProtocolException: Authorized clients cannot send packet 8
   at SkynetServer.Network.Client.OnPacketReceived(Byte id, Byte[] content) in /app/src/SkynetServer/Network/Client.cs:line 88
   at VSL.VSLSocket.OnPacketReceived(Byte id, Byte[] content)
Successfully sent packet {P0BChannelMessage: ContentId=2b ChannelId=caf922b466760441 MessageId=000000eb Flags=Unencrypted, NoSenderSync}
Client connection established with version 1.3
Starting to handle packet {P00ConnectionHandshake}
Successfully sent packet {P01ConnectionResponse}
Starting to handle packet {P08RestoreSession: AccountId=66b72445c88510e SessionId=11ad734154e558a}
Successfully sent packet {P09RestoreSessionResponse: ErrorCode=Success}
Successfully sent packet {P0FSyncFinished}

Additional context This bug is most likely a race condition and could be connected with channel actions as it only appears in this context with no other packets involved. One very interesting fact is, that VSL is no affected because the packets pass the integrity check. There might be multiple threads writing on the same VSL PacketBuffer.

Twometer commented 5 years ago

I don't think that VSL is not affected, as the only piece of code to ever send packet 0x0A is in the AddContact activity, which was not open at the time of the disconnect, yet the server read a 0x0A packet.

Twometer commented 5 years ago

It seems to appear when two clients change channel actions at the same time

Twometer commented 4 years ago

This should have been fixed with the introduction of TLS and rewrite of the network stack