Fix IndexError crash in TCP packet parser (packet types 115 and 131) - #168
Open
imwithsam wants to merge 1 commit into
Open
Fix IndexError crash in TCP packet parser (packet types 115 and 131)#168imwithsam wants to merge 1 commit into
imwithsam wants to merge 1 commit into
Conversation
Several device-index lookups (self.home_devices[home_id][idx]) in the packet_type == 115 and packet_type == 131 handlers were unguarded, unlike the equivalent lookup in the packet_type == 67 handler, which already checks the index against len(self.home_devices[home_id]) before using it. When Cync's cloud sends a packet whose device index isn't present in the locally cached home_devices list, this throws an uncaught IndexError: list index out of range, dropping the packet entirely (caught by the outer try/except and logged as 'Error parsing an incoming Cync TCP packet', with no further handling). Added the same bounds check already used in the packet_type == 67 handler to the 5 remaining unguarded lookups, so a packet referencing an out-of-range index is safely skipped instead of crashing the parse loop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
packet_type == 115andpacket_type == 131handlers in_read_tcp_messageseach have severalself.home_devices[home_id][idx]lookups with no bounds check.packet_type == 67handler right below them already guards its equivalent lookup withif int(packet[3]) < len(self.home_devices[home_id]):before indexing — this PR brings the other two handlers in line with that existing pattern.home_deviceslist throws an uncaughtIndexError: list index out of range, which gets caught by the outertry/exceptand logged asError parsing an incoming Cync TCP packet— the packet is silently dropped with no further handling.Observed this live on a real setup: the "state and brightness change" sub-handler (the one at
packet[21]) hit this exactIndexErrorrepeatedly on a bulb whose index wasn't (yet) inhome_devices[home_id]for that home. Confirmed the same unguarded pattern exists in 5 other spots across both handlers, so fixed all of them for consistency rather than just the one that happened to crash.Test plan
python3 -m py_compilepasses on the modified file