Metamask-extension: Instapage crashes with MetaMask

[danfinlay]

ConsenSys member Dean Ramadan reports that MetaMask causes web-page builder "Instapage" to crash within 10 seconds of starting up.

We should investigate & fix.

[tmashuang]

Could have been fixed in #2662. The PR will prevent MetaMask from injecting into XML scripts in 'Instapage' that has quite a few of.

[mateodelnorte]

This is currently a blocker for me too, but more importantly the bug presents itself as an instapage page just crashing the tab after 20 seconds. It will be very hard for a user to determine what's happening.

[danfinlay]

@mateodelnorte can you confirm if this is fixed in master? (we only partially rolled out the fix so far, so you may not have it yet).

[mateodelnorte]

Looks like I'm on v 3.13.2. I can go through the custom build for master tomorrow: https://github.com/MetaMask/metamask-extension/blob/master/docs/add-to-chrome.md

Gotta hit the sack so I can make a morning Star Wars for now!

[danfinlay]

Or just try again tomorrow, we should have a full roll-out by afternoon.

[danfinlay]

(I just got back, I'm resisting really hard dropping a bit that you wouldn't think is a spoiler but really is)

[mateodelnorte]

Don't do it! Don't crush my dreams!

On Dec 15, 2017 2:29 AM, "Dan Finlay" [e-mail] wrote:

(I just got back, I'm resisting really hard dropping a bit that you
wouldn't think is a spoiler but really is)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/MetaMask/metamask-extension/issues/2737#issuecomment-351933786,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAh3X6BF46F3ue0a0-SNilvEdX7YNVFRks5tAh_hgaJpZM4Q_z5l
.

[mateodelnorte]

Looks like this is still a bug. 🐞

[danfinlay]

Looks like this is still a bug. 🐞

That's why the issue is still open. We have a lot on our plate, please be patient. Pinging this thread every few days will not get it resolved faster.

[mateodelnorte]

You asked me to test on master, or wait until Sat to test with the soon-to-be deployed version. This was me testing and letting you know.

The work you guys are doing is very appreciated.

[danfinlay]

Oh got it, sorry I was impatient, been replying to users all morning.

[williamchong]

how can I reproduce this?
Started a 14days trial on instapage and clicked around, everything seems working

I am on metamask 3.13.3

[mateodelnorte]

Create and publish a page, like join.gridplus.io (this is a published
instapage). Wait 20 seconds to a minute. The tab will crash in chrome.

On Dec 19, 2017 1:14 AM, "William Chong" [e-mail] wrote:

how can I reproduce this?
Started a 14days trial on instapage and clicked around, everything seems
working

I am on metamask 3.13.3

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/MetaMask/metamask-extension/issues/2737#issuecomment-352650098,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAh3X-kdoslDwpRZxNYgxIXpBjRQ-8TBks5tB1RZgaJpZM4Q_z5l
.

[williamchong]

seems something to do with postMessage running into infinite loop?
probably need to ask Instapage to filter their message receiver, not sure

[gitcoinbot]

__This issue now has a funding of 0.065 ETH (52.31 USD) attached to it.__

• If you would like to work on this issue you can claim it here.
• If you've completed this issue and want to claim the bounty you can do so here
• Questions? Get help on the Gitcoin Slack
• $11511.51 more Funded OSS Work Available at: https://gitcoin.co/explorer

[ghost]

OK. This is due to a loop in the Instapage code caused by MetaMask's post-message-stream module and an iframe added to Instapages that will re-echo every message from post-message-stream to itself infinitely. You can view a reduced testcase at this gist which is directly ripped from the CDN where the iframe is being read from.

This iframe will always listen for every message, and then if it doesn't match its suspected data format, will copy the data from the old message into the new message and re-emit. You can crash the page without MetaMask, simply by emitting a similar message to the one MetaMask's post-message-stream emits:

window.postMessage(JSON.stringify({"target":"inpage","data":{"name":"publicConfig","data":{"networkVersion":"3"}}}), '*');

Or, more simply, kill that tab with the reduced testcase that has nothing to do with MetaMask:

window.postMessage(JSON.stringify({"a":1}),"*");

Because this message's data doesn't pass the error conditional, it will also be re-emitted with the data copied, over and over and over again until the tab OOM kills itself. It's an issue with Instapage, not really MetaMask. IMO, any extension that uses window.postMessage will have the same problems MetaMask is having.

Not sure the bounty attached here really applies, so I'm not claiming it unless someone here otherwise thinks I should.

Edit: This appears to be because MetaMask will inject its content scripts into all frames, including this Instapage iframe. Using the above payloads in the parent page will not reproduce the issue.

This may actually yield unintended interactions with third-party services such as advertisers that the user may not expect, that can then use an unlocked wallet and getAccounts to fingerprint users by Ethereum addresses. Not injecting the content scripts into the child iframe is a workaround in this case, but there is likely a greater privacy / information disclosure vulnerability here that should be discussed (yes, I know about the MetaMask warning that it's beta.)

[mateodelnorte]

Fantastic work. I'll relay this to the instapage team and report back.

On Dec 20, 2017 10:49 PM, "10a7" [e-mail] wrote:

OK. This is due to a loop in the Instapage code caused by MetaMask's
post-message-stream module and an iframe added to Instapages that will
re-echo every message from post-message-stream to itself infinitely. You
can view a reduced testcase at https://gist.github.com/10a7/
fe550ef35706e2ff1094b4731af9d352 directly ripped from the CDN where the
iframe is being read from.

This iframe will always listen for every message, and then if it doesn't
match its suspected data format, will copy the data from the old
message into the new message and re-emit. You can crash the page without
MetaMask, simply by emitting a similar message to the one MetaMask's
post-message-stream emits:

window.postMessage(JSON.stringify({"target":"inpage","data":{"name":"publicConfig","data":{"networkVersion":"3"}}}), '*');

Or, more simply, kill that tab with the reduced testcase that has nothing
to do with MetaMask:

window.postMessage(JSON.stringify({"a":1}),"*");

Because this message's data doesn't pass the error conditional, it will
also be re-emitted with the data copied, over and over and over again
until the tab OOM kills itself. It's an issue with Instapage, not really
MetaMask. Any extension that injects window.postMessage will have the same
problems MetaMask is having. MetaMask could not attach itself into child
iframes of the viewed document and it would work around this issue, but IMO
it's not the extension's fault.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/MetaMask/metamask-extension/issues/2737#issuecomment-353251292,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAh3X8bK7zYDhFY_YyJCwXWjZSNJGQaQks5tCdU2gaJpZM4Q_z5l
.

[mateodelnorte]

@10a7 Just to be clear, is the recommendation here that Metamask only inject into the parent document?

Seems the bug in instapage is worthy of reporting to them, as it may also sabotage other extensions using this technique. However, the simpler route could be updating Metamask – unless this is likely to cause large breakages for sites that are depending on injection into child iframes.

[danfinlay]

MetaMask injects into all frames because all frames mught want a web3 context. It sounds clear to me this should be fixed on the Instapage side.

[ghost]

@danfinlay Since there’s no bug in MetaMask here, can we repurpose the bug’s bounty to a MetaMask bug? I’m not sure how Gitcoin works. I technically solved the problem, but it’s not like I can make a pull request to fix a third-party site.

[danfinlay]

Yeah, I figure if you report the issue to Instapage, that would represent the most we can do here. If you can get this information to them, I think you'd have earned the bounty.

[ghost]

I filed a ticket with Instapage using their support center linking back to this bug. Considering “Bug” is one of the categories available on that ticket support system, I suspect this is the orthodox way of reporting these issues to them.

[kumavis]

Edit: This appears to be because MetaMask will inject its content scripts into all frames, including this Instapage iframe. Using the above payloads in the parent page will not reproduce the issue.

is this because instapage is replaying the message or bc the two metamasks in the page and iframe are sending messages back and forth?

@10a7

[ghost]

@kumavis It appears to be because the Instapage iframe is replaying the message inside itself. It picks up any message MetaMask sends, and then replays it, which then fails, and it replays the failed one, etc. If you take the iframe I have in the Gist above, you can reproduce the issue without MetaMask at all, and without the page being in an iframe using one of the payloads in the original report.

It’s an infinite loop in their error handling (their error handling throws, then replays the message in its error handler, which then rethrows, etc.) Anything that posts a message into this iframe will cause the issue. You can just crash the process faster by sending more than one message.

[ghost]

Instapage has replied on an internal ticket and has escalated the bug to their technical team.

[ghost]

Instapage has replied on an internal ticket and the bug has been filed with Instapage engineering.

[kumavis]

@10a7 did you claim the issue on gitcoin?

[owocki]

hey @10a7 can you please claim the issue on gitcoin ( https://gitcoin.co/funding/details?url=https://github.com/MetaMask/metamask-extension/issues/2737 ) and comment back here when you have?

message me on slack if you have any issues

[gitcoinbot]

__The funding of 0.065 ETH (43.55 USD) attached has been claimed by @10a7.__

@10a7, please leave a comment to let the funder (@owocki) and the other parties involved your implementation plan. If you don't leave a comment, the funder may expire your claim at their discretion.

• Learn more on the gitcoin issue page
• Questions? Get help on the Gitcoin Slack
• $10629.55 more Funded OSS Work Available at: https://gitcoin.co/explorer

[gitcoinbot]

__The funding of 0.065 ETH (43.55 USD) attached to this issue has been approved & issued to @10a7.__

• Learn more at on the gitcoin issue page
• Questions? Get help on the Gitcoin Slack
• $10586.0 more Funded OSS Work Available at: https://gitcoin.co/explorer

[ghost]

Thanks @kumavis. I actually broke Gitcoin which made it impossible for me to claim this issue, and didn't have enough gas in my MetaMask wallet to re-execute the Tx. @owocki came and saved the day.

[danfinlay]

@owocki this issue still appears open on gitcoin.

[danfinlay]

Oh, I see, it's listed as "fulfilled". Nevermind!

[ghost]

Instapage updated the ticket this morning and stated that this should be fixed on their end.

[danfinlay]

Thanks for the update! Can any users here confirm that? I'll reopen if users refute that it's fixed.