Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protect against malformed initial join room state #367

Closed
kegsay opened this issue Nov 7, 2023 · 1 comment · Fixed by #368
Closed

Protect against malformed initial join room state #367

kegsay opened this issue Nov 7, 2023 · 1 comment · Fixed by #368
Labels
bug Something isn't working

Comments

@kegsay
Copy link
Member

kegsay commented Nov 7, 2023

It is possible for Synapse to send back malformed sync v2 responses, which look like this:

{
    "join": {
        "!IcnzdHPPHhnQSIYddY:hs1": {
            "account_data": {
                "events": []
            },
            "ephemeral": {
                "events": []
            },
            "state": {
                "events": [
                    {
                        "content": {
                            "join_rule": "invite"
                        },
                        "event_id": "$-s3dp1zqApBugW6UOBWzGRFlRf1uvFKSqSeHVDl7I_w",
                        "origin_server_ts": 1699375767822,
                        "sender": "@user-1:hs1",
                        "state_key": "",
                        "type": "m.room.join_rules",
                        "unsigned": {
                            "age": 977
                        }
                    },
                    {
                        "content": {
                            "ban": 50,
                            "events": {
                                "m.room.avatar": 50,
                                "m.room.canonical_alias": 50,
                                "m.room.encryption": 100,
                                "m.room.history_visibility": 100,
                                "m.room.name": 50,
                                "m.room.power_levels": 100,
                                "m.room.server_acl": 100,
                                "m.room.tombstone": 100
                            },
                            "events_default": 0,
                            "historical": 100,
                            "invite": 0,
                            "kick": 50,
                            "redact": 50,
                            "state_default": 50,
                            "users": {
                                "@user-1:hs1": 100
                            },
                            "users_default": 0
                        },
                        "event_id": "$0rs4J6X4oQdER16GeFd5dVJnzuzZd7Gk3lO5n-ZidVk",
                        "origin_server_ts": 1699375767818,
                        "sender": "@user-1:hs1",
                        "state_key": "",
                        "type": "m.room.power_levels",
                        "unsigned": {
                            "age": 981
                        }
                    },
                    {
                        "content": {
                            "guest_access": "can_join"
                        },
                        "event_id": "$QumnUTkpI1r2HqBaBwVsR1M8RhnlRuyW6wHV5dh4Jek",
                        "origin_server_ts": 1699375767823,
                        "sender": "@user-1:hs1",
                        "state_key": "",
                        "type": "m.room.guest_access",
                        "unsigned": {
                            "age": 976
                        }
                    },
                    {
                        "content": {
                            "history_visibility": "shared"
                        },
                        "event_id": "$jtaiXEi4M_5z_gcPjSqmCTLr346jYtGcLriyPGsMINA",
                        "origin_server_ts": 1699375767822,
                        "sender": "@user-1:hs1",
                        "state_key": "",
                        "type": "m.room.history_visibility",
                        "unsigned": {
                            "age": 977
                        }
                    }
                ]
            },
            "summary": {},
            "timeline": {
                "events": [
                    {
                        "content": {
                            "creator": "@user-1:hs1",
                            "room_version": "10"
                        },
                        "event_id": "$ZVJ-2IpB4_Q_SekOHpfS5TYfXJsO09MOLjn7e9Mb0TU",
                        "origin_server_ts": 1699375767666,
                        "sender": "@user-1:hs1",
                        "state_key": "",
                        "type": "m.room.create",
                        "unsigned": {
                            "age": 1133
                        }
                    },
                    {
                        "content": {
                            "displayname": "user-1",
                            "membership": "join"
                        },
                        "event_id": "$wkt0vwF8jnp5-9PZWlcUJLqC_nrod_njGfd7J92TUB8",
                        "origin_server_ts": 1699375767774,
                        "sender": "@user-1:hs1",
                        "state_key": "@user-1:hs1",
                        "type": "m.room.member",
                        "unsigned": {
                            "age": 1025
                        }
                    },
                    {
                        "content": {
                            "avatar_url": null,
                            "displayname": "user-2",
                            "membership": "join"
                        },
                        "event_id": "$xXr5h3VwkV213gif8JEBzjWHKp6zzCl7H0dRsJKy7rY",
                        "origin_server_ts": 1699375768754,
                        "sender": "@user-2:hs2",
                        "state_key": "@user-2:hs2",
                        "type": "m.room.member",
                        "unsigned": {
                            "age": 8,
                            "prev_content": {
                                "displayname": "user-2",
                                "membership": "invite"
                            },
                            "prev_sender": "@user-1:hs1",
                            "replaces_state": "$k_kVHm4t3-ct_esCvXcuHDoY5PATx9SuppXkqeahU30"
                        }
                    }
                ],
                "limited": true,
                "prev_batch": "s5_3_0_1_1_1_1_2_0_2"
            },
            "unread_notifications": {
                "highlight_count": 0,
                "notification_count": 0
            }
        }
    }
}

This is malformed because the m.room.create event is objectively in the wrong place. It should be in state, not timeline. Unfortunately, this precise response causes a sequence of failures which ends up failing to process this room and the proxy not noticing it.

Currently the proxy will:

  • Attempt to parse state.
  • The Initialilse function fails to parse state due to lack of a create event. This will cause an internal.DataError to be returned as it is a problem with the data. This means "do not retry this sync request, skip over".
  • By returning an error from Initialise, we never look at the timeline: https://github.com/matrix-org/sliding-sync/blob/main/sync2/poller.go#L795
  • This means we never see the m.room.create event in the timeline, which could mean we actually process this room correctly.
  • Due to the DataError being returned, we now increment since, permanently losing the room.

To fix this, the proxy should:

  • Upon getting a DataError from Initialise, look for an m.room.create in the timeline.
  • If it exists, move it to state, along with other state events bar the last event.
  • Keep the last event in the timeline
  • Retry Initialise.
  • If that fails, give up. If it succeeds, log loudly that we have caught this.
@kegsay kegsay added the bug Something isn't working label Nov 7, 2023
@kegsay
Copy link
Member Author

kegsay commented Nov 7, 2023

xref matrix-org/complement#690

kegsay added a commit that referenced this issue Nov 8, 2023
Specifically, look for the create event in the timeline as this
has been seen in the wild on Synapse. Fixes #367.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant