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

Fix content spoofing in notifications #2307

Open
corrideat opened this issue Aug 18, 2024 · 0 comments
Open

Fix content spoofing in notifications #2307

corrideat opened this issue Aug 18, 2024 · 0 comments

Comments

@corrideat
Copy link
Member

corrideat commented Aug 18, 2024

Problem

PR #2046 introduced changes to templates.js that cause content spoofing.

The specific change is that the notification text, instead of including a username, now includes @${contractID}, with the substitution happening during rendering.

I don't think that this change is a good idea because it makes the rendering logic need to parse text strings and it can lead to at least some limited content spoofing.

In particular, if the notification text contains a valid @${contractID} string, that username might be rendered instead of the intended string. Granted, this is unlikely and therefore this issue isn't exactly high priority. However, that doesn't mean it shouldn't be fixed.

For example, if a notification includes the text Something @123 happened, it might be rendered as Something alice happened instead of the original text.
  

Solution

There are two ways to address this:

Option 1

Revert most of the changes to templates.js that introduce this behaviour to the old behaviour of looking up usernames at the point in time the notification is created. Then, revert the string substitution changes. However, make sure that the key used for notification storage includes the ID and not the username. The downside of this is that the rendered text will reflect an outdated username, if it should change.

Option 2

This would maybe be a 'better' solution, and it should also be applied to chatroom mentions. This involves changes to separate data from presentation, so that the rendering code doesn't need to parse text to extract usernames.

For example, the notification text (or chatroom mentions) could look something like ["this", " is some text and ", { type: "mention", contractID: "123" }, " said so." ], which would be rendered as this is some text and alice said so, if alice has the contract ID 123.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants