search

WeblateOrg / weblate

Web based localization tool with tight version control integration.
https://weblate.org/
GNU General Public License v3.0
4.62k stars 1.02k forks source link

potential security leak by notification emails #3186

Closed surli closed 4 years ago

surli commented 5 years ago

Describe the bug

As part of a process for preparing an upgrade, I'm checking the upgrade on my machine before doing it on prod. In order to stub the email server, I'm using fakesmtp so I can see the emails that are sent. At a point I clicked by mistake on Manage > Addons on a component, but the thing is I don't have the VCS locally when checking the upgrade, so of course I got an error about a missing property file.

Now the problem is that I got the notification emails for this error in fakesmtp, and it appears that it has not only be sent to the manager / admin, but also to users watching the project and being active (if I understand well).

So, now I'm wondering: would that be the case for any error happening when managing a component? IMO those errors should only be sent to some specific users, not to anyone since they might contain sensitive information: right now the emails contain a full path on the machine about the missing file. Even that, I'm not sure I want the users to get it...

To Reproduce Steps to reproduce the behavior:

  1. Run an instance of weblate without the VCS and with fakesmtp launched to gather the sent emails
  2. Go to Manage > Addons on a component

Expected behavior An error email is only sent to the to the admin.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/83276020-potential-security-leak-by-notification-emails?utm_campaign=plugin&utm_content=tracker%2F253393&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F253393&utm_medium=issues&utm_source=github).
nijel commented 5 years ago

Currently, anybody with access to the project can subscribe to the error events. They are also stored as alerts or visible in the history.

The notification should contain only relative path in the repository, which is also visible while translating, so it's not really private. Did you really get full path somewhere? Can you please share the error you got?

surli commented 5 years ago

AFAIU the path is not coming from weblate, but from the error message that is sent by notification email. The interface just displays that there was an internal error. But the mail provide lot more information, see: Capture d’écran_2019-11-06_14-41-23

surli commented 5 years ago

Currently, anybody with access to the project can subscribe to the error events. They are also stored as alerts or visible in the history.

Do you plan to change that on the future? at least to be able to hide them for regular users.

surli commented 5 years ago

So I dig up a bit, and it might also contains information related to custom addons. For example, few weeks ago we got an error with on of our script and email containing the following has been triggered with full paths clearly displayed:

[full_path]post_update.sh | Command '[u'[full_path]post_update.sh']' returned non-zero exit status 1
-- | --
Couldn't find the base translation file for this file mask: [xwiki-platform-core/xwiki-platform-annotations/xwiki-platform-annotation-ui/src/main/resources/AnnotationCode/Translations_*.properties]
nijel commented 5 years ago

I've improved the error handling in this particular case to include just the actual error message in the alert, see 023b62bbe4.

Generally this has not yet been considered as an issue. The alerts were added as an indication to the translators that something might be wrong with this translation and to serve this purpose, we can't hide such errors.

surli commented 5 years ago

The alerts were added as an indication to the translators that something might be wrong with this translation and to serve this purpose, we can't hide such errors.

I see the purpose of informing the translators that something might be wrong, but not really on being specific on what's wrong. Now the only case I encountered were git conflicts, problems with addons, or the file not found: that's not things the translators can handle.

I'd understand better if the error were about specific things to be handled by the translators.

nijel commented 5 years ago

The main motivation for this is Hosted Weblate with hundredths of open-source projects with different level of care. Even if the translator can not do anything about it, he should consider contributing to broken project or complain to its maintainer. In well controlled environment this might be not needed.

surli commented 5 years ago

OK I understand a bit more the main motivation, even if IMO it shouldn't be the translators job to inform the maintainer in those cases, but that's another debate :)

Anyway, thanks already for the first fix. I still think it could be nice on the long term to make it optional or have a little bit more possibility to customize those.

surli commented 5 years ago

Just for info, I had a new error this morning because of a lack of space on my instance and the error was displayed like that:

error: failed to write new configuration file /home/weblate/weblate/lib/python2.7/site-packages/data/vcs/xwiki-contrib/jw-player-macro/.git/config.lock (4)

I'm not fluent enough in python to understand your fix properly, so not sure it's covered :)

nijel commented 5 years ago

No, this one is not covered.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

nijel commented 4 years ago

I've fixed the most obvious cases, but I don't think we can ever address this 100%, there is always option the error message from called programs will leak more than what's necessary.