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.

Sign up for GitHub

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

Jump to bottom

Introduce a higher level DisplayName struct which helps to protect against impersonation via display names #4237

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Introduce a higher level DisplayName struct which helps to protect against impersonation via display names #4237

wants to merge 2 commits into from
+816 −109

Conversation

poljar
Copy link
Contributor

@poljar poljar commented Nov 8, 2024 •
edited
Loading

No description provided.

@poljar poljar requested a review from a team as a code owner November 8, 2024 13:33
@poljar poljar requested review from jmartinesp and removed request for a team November 8, 2024 13:33
@codecov Codecov
Copy link

codecov bot commented Nov 8, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 92.03540% with 9 lines in your changes missing coverage. Please review.

Project coverage is 85.05%. Comparing base (cefd5a2) to head (fbcf21d).

Files with missing lines Patch % Lines
...ates/matrix-sdk-base/src/deserialized_responses.rs 91.48% 4 Missing ⚠️
crates/matrix-sdk-base/src/sliding_sync/mod.rs 33.33% 2 Missing ⚠️
crates/matrix-sdk-base/src/store/ambiguity_map.rs 88.23% 2 Missing ⚠️
crates/matrix-sdk-sqlite/src/state_store.rs 95.65% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4237      +/-   ##
==========================================
+ Coverage   85.00%   85.05%   +0.04%     
==========================================
  Files         274      274              
  Lines       29945    30008      +63     
==========================================
+ Hits        25456    25524      +68     
+ Misses       4489     4484       -5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jmartinesp
jmartinesp approved these changes Nov 8, 2024
View reviewed changes
Copy link
Contributor

@jmartinesp jmartinesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

crates/matrix-sdk-base/src/deserialized_responses.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-base/src/deserialized_responses.rs Outdated Show resolved Hide resolved
@jmartinesp
Copy link
Contributor

It seems like Rust doesn't like the way the lazy regexes are implemented though.

@poljar
Copy link
Contributor Author

poljar commented Nov 8, 2024

It seems like Rust doesn't like the way the lazy regexes are implemented though.

Yeah, though we don't mutate things but I'll see how to implement this without any compiler complaints.

@poljar poljar requested a review from a team November 13, 2024 11:03
@poljar poljar force-pushed the poljar/homologlyph-pr branch 2 times, most recently from d4950a3 to 04115dd Compare November 14, 2024 16:12
poljar and others added 2 commits November 14, 2024 17:38
@poljar @dkasak
feat(base): Introduce a DisplayName struct
c1dae4f 
This patch introduces a struct that normalizes and sanitizes display
names. Display names can be a source of abuse and can contain characters
which might make it hard to distinguish one display name from the other.

This struct attempts to make it easier to protect against such abuse.

Changelog: Introduce a DisplayName struct which normalizes and sanitizes
display names.

Co-authored-by: Denis Kasak <[email protected]>
@poljar
fix: Use the DisplayName struct to protect against homoglyph attacks
fbcf21d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnjbvr bnjbvr bnjbvr left review comments

@Hywan Hywan Hywan left review comments

@dkasak dkasak dkasak left review comments

@jmartinesp jmartinesp jmartinesp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@poljar @jmartinesp @dkasak @Hywan @bnjbvr