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

Enigma: Redressing attack prevention - security background (#6450) #6759

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alecpl
Copy link
Member

@alecpl alecpl commented May 17, 2019

See the ticket for some screenshots and discussion.

@dereks
Copy link

dereks commented Aug 20, 2020

Thank you for submitting a Pull Request (PR) to the Roundcube GitHub project.

You are receiving this message because your PR has conflicts which need to be resolved. We are trying to catch up on our backlog of old PRs and get them merged in (where appropriate). Therefor, we request the following:

Step 1. Rebase from the latest Roundcube branch master into your PR.

git fetch upstream
git checkout your_feature_branch_name
git rebase upstream/master
git push -f origin your_feature_branch_name

Step 2. Re-test to make sure your new code still works as expected

Step 3. Comment back here once it has been tested and will merge cleanly.

Once this has been done we will treat it like a new Pull Request and consider it for acceptance.

Apologies for the inconvenience. Thank you for contributing to Roundcube!

@LBBO
Copy link

LBBO commented Dec 3, 2020

I'd like to state that this approach offers next to no security improvement (as far as I can tell by just skimming over the code), since it only modifies the box's appearance. An attacker can still include an HTML block using certain IDs and classes and all images will be set for them by Roundcube.

I agree with the opinion in #6450 that signature verification must be displayed outside of attacker's control in order to not compromise on security. Sure, the UX might suffer in some cases, but for most use cases people wouldn't tell the difference.

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

Successfully merging this pull request may close these issues.

3 participants