diff --git a/telethon/network/mtprotosender.py b/telethon/network/mtprotosender.py index a1a02909..16b50d00 100644 --- a/telethon/network/mtprotosender.py +++ b/telethon/network/mtprotosender.py @@ -505,6 +505,8 @@ class MTProtoSender: try: message = self._state.decrypt_message_data(body) + if message is None: + continue # this message is to be ignored except TypeNotFoundError as e: # Received object which we don't know how to deserialize self._log.info('Type %08x not found, remaining data %r', diff --git a/telethon/network/mtprotostate.py b/telethon/network/mtprotostate.py index 0fe9cc08..03026706 100644 --- a/telethon/network/mtprotostate.py +++ b/telethon/network/mtprotostate.py @@ -2,6 +2,7 @@ import os import struct import time from hashlib import sha256 +from collections import deque from ..crypto import AES from ..errors import SecurityError, InvalidBufferError @@ -10,6 +11,17 @@ from ..tl.core import TLMessage from ..tl.tlobject import TLRequest from ..tl.functions import InvokeAfterMsgRequest from ..tl.core.gzippacked import GzipPacked +from ..tl.types import BadServerSalt, BadMsgNotification + + +# N is not specified in https://core.telegram.org/mtproto/security_guidelines#checking-msg-id, but 500 is reasonable +MAX_RECENT_MSG_IDS = 500 + +MSG_TOO_NEW_DELTA = 30 +MSG_TOO_OLD_DELTA = 300 + +# Something must be wrong if we ignore too many messages at the same time +MAX_CONSECUTIVE_IGNORED = 10 class _OpaqueRequest(TLRequest): @@ -54,6 +66,9 @@ class MTProtoState: self.salt = 0 self.id = self._sequence = self._last_msg_id = None + self._recent_remote_ids = deque(maxlen=MAX_RECENT_MSG_IDS) + self._highest_remote_id = 0 + self._ignore_count = 0 self.reset() def reset(self): @@ -64,6 +79,9 @@ class MTProtoState: self.id = struct.unpack('q', os.urandom(8))[0] self._sequence = 0 self._last_msg_id = 0 + self._recent_remote_ids.clear() + self._highest_remote_id = 0 + self._ignore_count = 0 def update_message_id(self, message): """ @@ -134,6 +152,8 @@ class MTProtoState: """ Inverse of `encrypt_message_data` for incoming server messages. """ + now = time.time() + self.time_offset # get the time as early as possible, even if other checks make it go unused + if len(body) < 8: raise InvalidBufferError(body) @@ -159,6 +179,16 @@ class MTProtoState: raise SecurityError('Server replied with a wrong session ID') remote_msg_id = reader.read_long() + + if remote_msg_id % 2 != 1: + raise SecurityError('Server sent an even msg_id') + + # Only perform the (somewhat expensive) check of duplicate if we did receive a lower ID + if remote_msg_id <= self._highest_remote_id and remote_msg_id in self._recent_remote_ids: + self._log.warning('Server resent the older message %d, ignoring', remote_msg_id) + self._count_ignored() + return None + remote_sequence = reader.read_int() reader.read_int() # msg_len for the inner object, padding ignored @@ -167,8 +197,39 @@ class MTProtoState: # reader isn't used for anything else after this, it's unnecessary. obj = reader.tgread_object() + # "Certain client-to-server service messages containing data sent by the client to the + # server (for example, msg_id of a recent client query) may, nonetheless, be processed + # on the client even if the time appears to be "incorrect". This is especially true of + # messages to change server_salt and notifications about invalid time on the client." + # + # This means we skip the time check for certain types of messages. + if obj.CONSTRUCTOR_ID not in (BadServerSalt.CONSTRUCTOR_ID, BadMsgNotification.CONSTRUCTOR_ID): + remote_msg_time = remote_msg_id >> 32 + time_delta = now - remote_msg_time + + if time_delta > MSG_TOO_OLD_DELTA: + self._log.warning('Server sent a very old message with ID %d, ignoring', remote_msg_id) + self._count_ignored() + return None + + if -time_delta > MSG_TOO_NEW_DELTA: + self._log.warning('Server sent a very new message with ID %d, ignoring', remote_msg_id) + self._count_ignored() + return None + + self._recent_remote_ids.append(remote_msg_id) + self._highest_remote_id = remote_msg_id + self._ignore_count = 0 + return TLMessage(remote_msg_id, remote_sequence, obj) + def _count_ignored(self): + # It's possible that ignoring a message "bricks" the connection, + # but this should not happen unless there's something else wrong. + self._ignore_count += 1 + if self._ignore_count >= MAX_CONSECUTIVE_IGNORED: + raise SecurityError('Too many messages had to be ignored consecutively') + def _get_new_msg_id(self): """ Generates a new unique message ID based on the current