Project

General

Profile

Bug #16421

Electrum Phishing Attack - Upstream Fix Committed

Added by tailshark 10 months ago. Updated 2 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
Start date:
02/05/2019
Due date:
% Done:

100%

Feature Branch:
bugfix/16421-fix-electrum+force-all-tests
Type of work:
Code
Blueprint:
Starter:
Affected tool:
Electrum

Description

As part of this ticket, reintroduce the test case i.e. revert f092b0d6f268a12550283e3a510f0455055ca1d9.

Initial report:

I was using Tails (newest version) and stumbled over this a few hours ago.

When broadcasting a Bitcoin transaction it would come back telling me to manually upgrade Electrum with a link. I thought this was suspicious as the response was rich text and my hygiene (cyber or otherwise) is amazing.

Did a little digging and this:
https://github.com/spesmilo/electrum/issues/4968

Bottom line: Attacker electrum nodes in the wild are able to send custom responses to Electrum <v3.3.3. Tails looks like it's at v3.1.3 at present. Electrum devs responded with a counter-move. They started upgrading Electrum nodes to authorize your transaction but shout at you for using an older version.

Current user experience: At this time every Electrum transaction on Tails shouts at me. It's either the phishing response trying to bait me into installing the backdoored Electrum (and the transaction fails) OR it's a legitimate Electrum node that authorizes the transaction but tells me I'm on a vulnerable version.

At this time it looks like the attack requires user participation to manually go and install stuff from the attacker site(s). I'm not sure how many Tails users this would actually pwn since Tails users are here for a reason. But at the very least it might freak people out. I checked all the doors & hatchets myself when seeing the phising response for the first time.

Thought I would share before you get a ticket like this one:

"I'm lose ~12 BTC ~ $42k, from an UPDATE SHOW ME ON 3.3.3 OFFICIAL !!!! my family going to dead #5064"

https://github.com/spesmilo/electrum/issues/5064


Related issues

Related to Tails - Bug #16564: Consider shipping Electrum as an AppImage Resolved 03/15/2019
Related to Tails - Feature #16565: Mention Electrum updates on the doc/known issues page Resolved 03/16/2019
Blocks Tails - Bug #16969: "Electrum starts" test step is broken on Buster In Progress
Blocks Tails - Feature #16209: Core work: Foundations Team Confirmed
Blocks Tails - Bug #9732: Orca cannot work with Electrum Confirmed 07/13/2015
Blocks Tails - Bug #15483: Update Electrum configuration file Resolved 03/31/2018

Associated revisions

Revision f092b0d6 (diff)
Added by intrigeri 4 months ago

Test suite: remove totally broken Electrum scenario (refs: #16421)

Given Electrum is broken in Tails currently, this scenario gives us zero
valuable information but its constant failure has a cost. Let's can bring it
back once we have a working Electrum again.

Revision 46d8529c (diff)
Added by segfault 3 months ago

Use Electrum from sid (refs: #16421)

Revision ccb389bc (diff)
Added by segfault 3 months ago

Use Electrum from sid (refs: #16421)

Revision 4645d460 (diff)
Added by segfault 3 months ago

Revert "Test suite: remove totally broken Electrum scenario (refs: #16421)"

This reverts commit f092b0d6f268a12550283e3a510f0455055ca1d9.

We have a working Electrum version again.

Revision ae8f4b8c
Added by intrigeri 2 months ago

Merge remote-tracking branch 'origin/bugfix/16421-fix-electrum+force-all-tests' into devel (Closes: #16421)

History

#1 Updated by mercedes508 10 months ago

  • Assignee set to intrigeri
  • Type of work changed from Code to Research

Already received a report about this issue. Not sure what we can do from a Tails perspective as said here https://labs.riseup.net/code/issues/12545#note-5

#2 Updated by intrigeri 10 months ago

  • Assignee changed from intrigeri to s7r

Dear s7r, can you please triage this?

#3 Updated by s7r 10 months ago

  • Status changed from New to Confirmed
  • Priority changed from Normal to Elevated
  • Type of work changed from Research to Wait

This is confirmed.

Some important notes:

- there is no vulnerability in the application itself. Nothing can be exploited. The only bug is that the arbitrary error message sent by the malicious server is displayed as rich text by QT.

- because of how electrum server peer discovery works, there is nothing that can stop the sybil attack or filter the servers. There's no authoritative directory which assigns flags or reputations to servers. Any client using auto-connect or random server has the N % chances to run into a malicious server where N = the % of the malicious servers in the server pool.

- it is a phishing attack. if the users use a trusted server, or a honest server, they are unaffected. If the users use a malicious server, get the error but don't follow up and don't install Electrum from untrusted sources, and just simply switch servers, they remain unaffected.

- the fix upstream in 3.3.3 ONLY renders error messages to plain text instead of rich text, and doesn't allow arbitrary messages but strict error codes.

- in Tails it's not trivial to install something from other sources, so I aim to think Tails users are OK.

At the moment, the network of Electrum servers is sybil-ed with malicious peers, there are so so many.

ElectrumX (the Electrum server implementation) 1.9.3 was tagged which:

a) filters and identifies most malicious peers (servers) and does not further broadcast them to clients;

b) uses the same "bug" to display a WARNING that the users should upgrade, even thus the transaction was successfully sent.

This was done because, users simply don't upgrade in the wild, and remain vulnerable to the phishing scam.

Electrum's Debian package maintainer had some problems that did not allow him to work on Debian, and when I last discussed I understood that work will be resumed at mid February, which is soon. It's going to be tight with the Buster freeze, but let's see what happens.

This is what we are waiting for in order to close all Electrum tickets. There is nothing much we can do in Tails, except display a notification on the website, or maybe even in Tails itself, that all Electrum users will either:

a) get a phishing message that will advice them to install a backdoored Electrum from an untrusted source, message that should be ignored.

b) get a warning message that the transaction was sent, but the version of Electrum used is vulnerable. This will be fixed in another release.

c) provide some trusted onion Electrum servers so that they don't have to go through many servers until they find one that broadcasts their transaction, since the Electrum server pool is heavily sybiled at this moment.

Comments from more people needed here, to see what's the best way here.

#4 Updated by tailshark 10 months ago

@s7r - Sounds good. I was going to ask who maintains bringing Electrum security fixes to the Debian mainline. I had some concerns this might sit in rotation until June~ or whenever Buster releases.

#5 Updated by sajolida 9 months ago

  • Related to Feature #16565: Mention Electrum updates on the doc/known issues page added

#6 Updated by sajolida 9 months ago

  • Related to Bug #16564: Consider shipping Electrum as an AppImage added

#7 Updated by sajolida 9 months ago

  • Related to Feature #16565: Mention Electrum updates on the doc/known issues page added

#8 Updated by sajolida 9 months ago

  • Related to deleted (Feature #16565: Mention Electrum updates on the doc/known issues page)

#9 Updated by mercedes508 9 months ago

f798abc6-1dd6-2fde-7c5c-50e71bfd5c82
1031575bee8e5ea9976c2a6d259dfb4d
eedcb659-51fc-62a3-3d21-b4bb7bd65614

#10 Updated by intrigeri 8 months ago

  • QA Check set to Info Needed
  • Type of work changed from Wait to Communicate

Provided guidance on the thread (https://lists.autistici.org/thread/20190320.121811.e9a13ab3.en.html), we'll decide how to proceed once s7r has answered the questions we have :)

#11 Updated by intrigeri 8 months ago

  • Target version set to Tails_3.14

#12 Updated by CyrilBrulebois 7 months ago

  • Target version changed from Tails_3.14 to Tails_3.15

#13 Updated by intrigeri 6 months ago

  • QA Check deleted (Info Needed)

#14 Updated by CyrilBrulebois 5 months ago

  • Target version changed from Tails_3.15 to Tails_3.16

#15 Updated by intrigeri 4 months ago

  • Blocks Bug #16969: "Electrum starts" test step is broken on Buster added

#16 Updated by intrigeri 4 months ago

  • Status changed from Confirmed to In Progress

#17 Updated by intrigeri 4 months ago

  • Description updated (diff)
  • Status changed from In Progress to Confirmed

#18 Updated by CyrilBrulebois 3 months ago

  • Target version changed from Tails_3.16 to Tails_3.17

#19 Updated by intrigeri 3 months ago

#20 Updated by intrigeri 3 months ago

  • Assignee deleted (s7r)
  • Target version changed from Tails_3.17 to Tails_4.0
  • Type of work changed from Communicate to Code

Electrum 3.3.8-0.1 is now in sid. It installs just fine on Buster. Additional dependencies that need to be pulled from sid too: python3-aiohttp-socks, python3-aiorpcx, python3-electrum.

For Tails 4.0, let's try to upgrade to this version and reintroduce + fix the corresponding test cases (#16969).

Implementation-wise, I'm undecided whether it's better to:

  • install it from sid ⇒ increased risk of breakage happening just before a Tails code freeze
  • install it from Bullseye ⇒ there's no security support for testing so it'll be our job to monitor whether security fixes landed in sid but did not migrate to testing yet; we have no mechanism in place for this; we already have this problem in other areas (#14728) but it may not be a sufficient reason to make it even worse

Thoughts?

#21 Updated by intrigeri 3 months ago

  • Blocks Bug #9732: Orca cannot work with Electrum added

#22 Updated by segfault 3 months ago

intrigeri wrote:

Electrum 3.3.8-0.1 is now in sid. It installs just fine on Buster. Additional dependencies that need to be pulled from sid too: python3-aiohttp-socks, python3-aiorpcx, python3-electrum.

For Tails 4.0, let's try to upgrade to this version and reintroduce + fix the corresponding test cases (#16969).

Implementation-wise, I'm undecided whether it's better to:

  • install it from sid ⇒ increased risk of breakage happening just before a Tails code freeze

Given that the Electrum Debian maintainer didn't find the time to work on this pressing issue, I don't think it's likely that there will be more updates in Debian soon. So the risk that a new package is uploaded right before a Tails code freeze seems low to me. And even if this happens, the fix (switching to Bullseye) seems trivial to me.

#23 Updated by segfault 3 months ago

  • Status changed from Confirmed to In Progress
  • Assignee set to segfault

#24 Updated by segfault 3 months ago

  • Feature Branch set to bugfix/16421-fix-electrum

On the feature branch, Electrum is installed from Sid. It seems to work: I can start it and create a wallet. I haven't used Electrum before, so I don't know what else I can test. @s7r, maybe you want to have a look? You can download the image here:

https://nightly.tails.boum.org/build_Tails_ISO_bugfix-16421-fix-electrum/lastSuccessful/archive/build-artifacts/tails-amd64-bugfix_16421-fix-electrum-4.0-20190920T1734Z-3bba768b94+devel@fe2c7513ee.img

#25 Updated by segfault 3 months ago

I'm not sure if our notifications work if the @NAME is not at the beginning of the line, so:
@s7r, could you test if Electrum works as expected in the above image?

#26 Updated by s7r 2 months ago

Hello,

I am trying to test, but when I go to the link to download the image I get a 404 Not Found error.
Did it become invalid during then?

#27 Updated by segfault 2 months ago

s7r wrote:

Hello,

I am trying to test, but when I go to the link to download the image I get a 404 Not Found error.
Did it become invalid during then?

Indeed that's case. Here is a new link: https://nightly.tails.boum.org/build_Tails_ISO_bugfix-16421-fix-electrum/lastSuccessful/archive/latest.img

#28 Updated by s7r 2 months ago

I have tested it from:

It works perfectly. We are being notified about persistence.
I have actually simulated some transactions to send / receive with it on top and with it minimized in the background and the notifications in the top bar work just fine (Electrum sent XXX or received XXX). I have generated both Segwith (new type) wallet as well as legacy wallet. I have tested the on the fly config file automated update (Electrum implemented a new config_version parameter and it is able to identify the current config file version and update it to the daemon version requirements, ignore deprecated features, etc.) and it works perfectly as well. I will comment more about this on #15483 about this.

All tests pass AFAICT.

#29 Updated by segfault 2 months ago

  • Status changed from In Progress to Needs Validation
  • Assignee deleted (segfault)

s7r wrote:

I have tested it from:

It works perfectly. We are being notified about persistence.
I have actually simulated some transactions to send / receive with it on top and with it minimized in the background and the notifications in the top bar work just fine (Electrum sent XXX or received XXX). I have generated both Segwith (new type) wallet as well as legacy wallet. I have tested the on the fly config file automated update (Electrum implemented a new config_version parameter and it is able to identify the current config file version and update it to the daemon version requirements, ignore deprecated features, etc.) and it works perfectly as well. I will comment more about this on #15483 about this.

All tests pass AFAICT.

Great, thanks!

#30 Updated by intrigeri 2 months ago

  • Blocks Bug #15483: Update Electrum configuration file added

#31 Updated by intrigeri 2 months ago

We are being notified about persistence.

To make sure I understand: did you test with persistence disabled, and thus get the expected "Persistence is disabled for Electrum" confirmation dialog?
Or did you experience anything else?

#32 Updated by s7r 2 months ago

intrigeri wrote:

To make sure I understand: did you test with persistence disabled, and thus get the expected "Persistence is disabled for Electrum" confirmation dialog?
Or did you experience anything else?

Yes, I tried with both persistence disabled and enabled. When I tried with persistence disabled I got the expected "Persistence is disabled for Electrum" confirmation dialog. So it works as it should.

#33 Updated by intrigeri 2 months ago

Yes, I tried with both persistence disabled and enabled. When I tried with persistence disabled I got the expected "Persistence is disabled for Electrum" confirmation dialog. So it works as it should.

Thanks!

#34 Updated by intrigeri 2 months ago

  • Assignee set to intrigeri
  • Feature Branch changed from bugfix/16421-fix-electrum to bugfix/16421-fix-electrum+force-all-tests

I've merged current devel into this branch to repair its build.

I've renamed the branch so that the fragile scenario reintroduced by this branch is run on Jenkins: that's the only automated scenario we have that actually tests Electrum (the other scenario merely tests the warning dialog that we display when starting Electrum without persistence).

I'll come back to it tomorrow, once Jenkins has tested build reproducibility and has run this scenario.

#35 Updated by intrigeri 2 months ago

  • Status changed from Needs Validation to In Progress
  • Assignee changed from intrigeri to segfault

intrigeri wrote:

I've renamed the branch so that the fragile scenario reintroduced by this branch is run on Jenkins: that's the only automated scenario we have that actually tests Electrum (the other scenario merely tests the warning dialog that we display when starting Electrum without persistence).

Unfortunately, that newly reintroduced scenario fails on Jenkins.

I'll come back to it tomorrow, once Jenkins has tested build reproducibility

This fails as well. I believe that's because a translatable string was changed on the branch and it would be fixed by refreshing the PO files.

#36 Updated by segfault 2 months ago

  • Assignee deleted (segfault)

Maybe someone else wants to look into why the test fails.

#37 Updated by segfault 2 months ago

  • Priority changed from Elevated to Normal

#38 Updated by segfault 2 months ago

Since Electrum seems to work fine, if we can't get the test scenario to work in time for 4.0~rc1, we could remove it again (and file a ticket to reintroduce it).

#39 Updated by intrigeri 2 months ago

Maybe someone else wants to look into why the test fails.

See also #16969, which was originally filed about this test failure :)

#40 Updated by segfault 2 months ago

  • Assignee set to segfault

#41 Updated by segfault 2 months ago

  • Status changed from In Progress to Needs Validation
  • Assignee deleted (segfault)

I couldn't fix #16969, so I removed the test from the branch.

#42 Updated by intrigeri 2 months ago

  • Assignee set to intrigeri

#43 Updated by intrigeri 2 months ago

segfault wrote:

I couldn't fix #16969, so I removed the test from the branch.

I can't see this in the topic branch so I'll do the revert before merging. I hope bugfix/16421-fix-electrum+force-all-tests is what I'm supposed to review & merge.

Code looks good. I've triggered a build and will merge once https://jenkins.tails.boum.org/job/reproducibly_build_Tails_ISO_bugfix-16421-fix-electrum-force-all-tests/ is happy.

#44 Updated by intrigeri 2 months ago

  • Status changed from Needs Validation to Resolved
  • % Done changed from 0 to 100

#45 Updated by segfault 2 months ago

intrigeri wrote:

segfault wrote:

I couldn't fix #16969, so I removed the test from the branch.

I can't see this in the topic branch so I'll do the revert before merging. I hope bugfix/16421-fix-electrum+force-all-tests is what I'm supposed to review & merge.

Oops, the branch I removed the test from was bugfix/16421-fix-electrum. Anyway, I just checked that that branch didn't have anything else that bugfix/16421-fix-electrum+force-all-tests doesn't have, so we're good.

Also available in: Atom PDF