chorus icon indicating copy to clipboard operation
chorus copied to clipboard

Potential future vulnerability in polyproto in Chorus

Open bitfl0wer opened this issue 5 months ago • 2 comments

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.

bitfl0wer avatar Jan 23 '24 22:01 bitfl0wer