Potential future vulnerability in polyproto in Chorus

[bitfl0wer]

There might be a future security vulnerability in Chorus, if we implement polyproto into Chorus without the correct considerations.

For example:

Lets say we have a Relationship object. PartialEq on Relationship is defined as such:

pub struct Relationship {
    pub id: Snowflake,
    #[serde(rename = "type")]
    pub relationship_type: RelationshipType,
    pub nickname: Option<String>,
    pub user: Shared<PublicUser>,
    pub since: Option<DateTime<Utc>>,
}

impl PartialEq for Relationship {
    fn eq(&self, other: &Self) -> bool {
        self.id == other.id
            && self.relationship_type == other.relationship_type
            && self.since == other.since
            && self.nickname == other.nickname
    }
}

Note, that the user field is not compared. This is because the User is behind an RwLock, and therefore the lock would have to be acquired to do an equality comparison on the User object. This is fine in our current context, and PartialEq documents that this is a possibility.

However; the id field of Relationship is the ID of the target user, i.e. the User the Relationship is described to. On Discord, Snowflake IDs are unique. In a federated context, this does not hold true, and an attacker could, if no change is made, craft a user object with the same Snowflake ID as another user, and possibly force a relationship with someone through that.

This needs to be considered when implementing polyproto into Chorus.

[erkinalp]

the User is behind an RwLock

Interesting design choice there.

[bitfl0wer]

Elaborate? Also, this is off-topic for this issue.

the User is behind an RwLock

Interesting design choice there.