-
-
Notifications
You must be signed in to change notification settings - Fork 79
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 RTLO attacks #3609
Conversation
This check is actually sender-wise. So attacker with old or no deltachat-client could still create falsy messages. The check should thus be on the receiving side, right? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is also LEFT-TO-RIGHT ISOLATE, RIGHT-TO-LEFT ISOLATE and the probably related FIRST STRONG ISOLATE and POP DIRECTIONAL ISOLATE, we probably want to strip them as well - but @Septias please double-check, i did no dive deep into that.
also, i agree, the receiving side is more important than tweaking sending; maybe altering sending is even superfluous then.
some tests would be nice as well, all in all, this is already a nice pr, however :)
707e0db
to
bdc434f
Compare
Makes sense to also strip them on the sending side for none-deltachat recepients I guess. @r10s |
probably okay, also as otherwise things will be displayed differently for sender and receiver. but if in doubt, receiving side is far more important. |
At the moment I am not quite sure how to implement my solution, maybe you can help me @r10s @hpk42 @Hocuri @link2xt.
The displayed problem is probably just one of many cases though because The other way to secure messages would be to check every occurrence of
and add security measures at the places where it is needed at last. |
Sorry that I didn't respond for such a long time.
If it's only in the displayname (like, I'm not sure what to do about email addresses - stripping characters can mean that we treat two different email addresses as the same, leading to new possible attacks. Completely treating them as invalid would be possible, but break completely break the app for anyone using such an email address, and make it impossible to communicate with such people.
Doing this in |
dac8748
to
961815d
Compare
4e90f11
to
9837040
Compare
707f906
to
9fa3353
Compare
can you take a look at this @link2xt so we can finally close this long open pr? |
Whats being sanitized:
improve_single_line_input
&add_or_lookup
&create
add_or_lookup
create_or_lookup_group
&create_group_chat
)receive_imf
)add_parts
create_status_update_record
)closes #3479