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

Bug: "oversized vector allocation" due to large transaction (lots of witness inputs) #783

Closed
C-Otto opened this issue Nov 1, 2022 · 20 comments
Assignees
Labels
bug Something isn't working p2p

Comments

@C-Otto
Copy link

C-Otto commented Nov 1, 2022

A recent transaction broke lnd (Lightning Network) and mempool.space (block explorer). Based on reports on Twitter electrs is also affected, but I didn't check myself.

"oversized vector allocation"

btcsuite/btcd#1906

@C-Otto C-Otto added the bug Something isn't working label Nov 1, 2022
@oliverlj
Copy link

oliverlj commented Nov 1, 2022

I confirm it is broken :

Starting electrs 0.9.4 on aarch64 linux with Config { network: Bitcoin, db_path: "/data/db/bitcoin", daemon_dir: "/data/.bitcoin", daemon_auth: CookieFile("/data/.bitcoin/.cookie"), daemon_rpc_addr: 10.21.21.8:8332, daemon_p2p_addr: 10.21.21.8:8333, electrum_rpc_addr: 0.0.0.0:50001, monitoring_addr: 127.0.0.1:4224, wait_duration: 10s, jsonrpc_timeout: 15s, index_batch_size: 10, index_lookup_limit: None, reindex_last_blocks: 0, auto_reindex: true, ignore_mempool: false, sync_once: false, disable_electrum_rpc: false, server_banner: "Umbrel Electrs (0.9.4-build-2)", args: [] }
[2022-11-01T11:45:01.791Z INFO  electrs::metrics::metrics_impl] serving Prometheus metrics on 127.0.0.1:4224
[2022-11-01T11:45:01.791Z INFO  electrs::server] serving Electrum RPC on 0.0.0.0:50001
[2022-11-01T11:45:01.957Z INFO  electrs::db] "/data/db/bitcoin": 151 SST files, 37.456491275 GB, 4.696187474 Grows
[2022-11-01T11:45:07.042Z INFO  electrs::chain] loading 761248 headers, tip=0000000000000000000640e114b9e6272b9688178273e193ef957e02fad1069f
[2022-11-01T11:45:11.850Z INFO  electrs::chain] chain updated: tip=0000000000000000000640e114b9e6272b9688178273e193ef957e02fad1069f, height=761248
[2022-11-01T11:45:12.293Z INFO  electrs::index] indexing 10 blocks: [761249..761258]
thread 'p2p_loop' panicked at 'invalid message: OversizedVectorAllocation { requested: 12000072, max: 4000000 }', src/p2p.rs:250:70
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[2022-11-01T11:45:12.353Z INFO  electrs::db] closing DB at /data/db/bitcoin
Error: electrs failed

Caused by:
    0: sync failed
    1: failed to get block 000000000000000000070742427fa10ec3c66d006160155a704d9f56d090a3ea
    2: receiving on an empty and disconnected channel

@0xB10C
Copy link

0xB10C commented Nov 1, 2022

My electrs 0.9.9 on x86_64 linux seems to be unaffected.

@modl21
Copy link

modl21 commented Nov 1, 2022

My electrs v0.9.8 seems unaffected as well.

@romanz romanz self-assigned this Nov 1, 2022
@elkimek
Copy link

elkimek commented Nov 1, 2022

My electrs v0.9.8 seems unaffected as well.

Same here on Nodl but bricked on another implementation.

@Kixunil
Copy link
Contributor

Kixunil commented Nov 1, 2022

This could be inside rust-bitcoin but we added a protection recently so maybe not. Will investigate.

@steepdawn974
Copy link

Issue confirmed electrs 0.9.7 on x86_64 linux

Nov 01 12:55:38 shallan electrs[3991523]: Starting electrs 0.9.7 on x86_64 linux with Config { network: Bitcoin, db_path: "/mnt/hdd/app-storage/electrs/db/bitcoin", daemon_dir: "/home/electrs/.bitcoin", daemon_auth: UserPass("raspibolt", "<sensitive>"), daemon_rpc_addr: 127.0.0.1:8332, daemon_p2p_addr: 127.0.0.1:8333, electrum_rpc_addr: 0.0.0.0:50001, monitoring_addr: 127.0.0.1:4224, wait_duration: 10s, jsonrpc_timeout: 15s, index_batch_size: 10, index_lookup_limit: None, reindex_last_blocks: 0, auto_reindex: true, ignore_mempool: false, sync_once: false, disable_electrum_rpc: false, server_banner: "Welcome to electrs v0.9.7 - the Electrum Rust Server on your RaspiBlitz", args: [] }
Nov 01 12:55:38 shallan electrs[3991523]: [2022-11-01T12:55:38.681Z INFO  electrs::metrics::metrics_impl] serving Prometheus metrics on 127.0.0.1:4224
Nov 01 12:55:38 shallan electrs[3991523]: [2022-11-01T12:55:38.681Z INFO  electrs::server] serving Electrum RPC on 0.0.0.0:50001
Nov 01 12:55:38 shallan electrs[3991523]: [2022-11-01T12:55:38.869Z INFO  electrs::db] "/mnt/hdd/app-storage/electrs/db/bitcoin": 150 SST files, 37.595860264 GB, 4.696187475 Grows
Nov 01 12:55:41 shallan electrs[3991523]: [2022-11-01T12:55:41.288Z INFO  electrs::chain] loading 761248 headers, tip=0000000000000000000640e114b9e6272b9688178273e193ef957e02fad1069f
Nov 01 12:55:43 shallan electrs[3991523]: [2022-11-01T12:55:43.678Z INFO  electrs::chain] chain updated: tip=0000000000000000000640e114b9e6272b9688178273e193ef957e02fad1069f, height=761248
Nov 01 12:55:43 shallan electrs[3991523]: [2022-11-01T12:55:43.695Z INFO  electrs::index] indexing 17 blocks: [761249..761265]
Nov 01 12:55:43 shallan electrs[3991523]: thread 'p2p_loop' panicked at 'invalid message: OversizedVectorAllocation { requested: 12000072, max: 4000000 }', src/p2p.rs:250:70
Nov 01 12:55:43 shallan electrs[3991523]: note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Nov 01 12:55:43 shallan electrs[3991523]: [2022-11-01T12:55:43.706Z INFO  electrs::db] closing DB at /mnt/hdd/app-storage/electrs/db/bitcoin
Nov 01 12:55:43 shallan electrs[3991523]: Error: electrs failed
Nov 01 12:55:43 shallan electrs[3991523]: Caused by:
Nov 01 12:55:43 shallan electrs[3991523]:     0: sync failed
Nov 01 12:55:43 shallan electrs[3991523]:     1: failed to get block 000000000000000000070742427fa10ec3c66d006160155a704d9f56d090a3ea
Nov 01 12:55:43 shallan electrs[3991523]:     2: receiving on an empty and disconnected channel
Nov 01 12:55:43 shallan systemd[1]: electrs.service: Main process exited, code=exited, status=1/FAILURE
Nov 01 12:55:43 shallan systemd[1]: electrs.service: Failed with result 'exit-code'.
Nov 01 12:55:43 shallan systemd[1]: electrs.service: Consumed 4.888s CPU time.

@yanascz
Copy link

yanascz commented Nov 1, 2022

Upgrading electrs to 0.9.9 fixed this for me as well on aarch64.

Nov 01 13:54:26 xyz electrs[1866423]: [2022-11-01T12:54:26.626Z INFO  electrs::server] serving Electrum RPC on 127.0.0.1:50001
Nov 01 13:54:26 xyz electrs[1866423]: [2022-11-01T12:54:26.690Z INFO  electrs::db] "/var/lib/electrs/db/bitcoin": 158 SST files, 37.636174394 GB, 4.696187479 Grows
Nov 01 13:54:30 xyz electrs[1866423]: [2022-11-01T12:54:30.565Z INFO  electrs::chain] loading 761248 headers, tip=0000000000000000000640e114b9e6272b9688178273e193ef957e02fad1069f
Nov 01 13:54:34 xyz electrs[1866423]: [2022-11-01T12:54:34.225Z INFO  electrs::chain] chain updated: tip=0000000000000000000640e114b9e6272b9688178273e193ef957e02fad1069f, height=761248
Nov 01 13:54:34 xyz electrs[1866423]: [2022-11-01T12:54:34.262Z INFO  electrs::index] indexing 17 blocks: [761249..761265]
Nov 01 13:54:35 xyz electrs[1866423]: [2022-11-01T12:54:35.049Z INFO  electrs::chain] chain updated: tip=000000000000000000051ca14f3ea01d5672b44d9ef3c95587afc0e3bd39aad6, height=761265
Nov 01 13:58:09 xyz electrs[1866423]: [2022-11-01T12:58:09.432Z INFO  electrs::index] indexing 1 blocks: [761266..761266]
Nov 01 13:58:09 xyz electrs[1866423]: [2022-11-01T12:58:09.504Z INFO  electrs::chain] chain updated: tip=00000000000000000003ac39882e5127ce163ee6bdca75d4b9bcf806c83afe56, height=761266

@Kixunil
Copy link
Contributor

Kixunil commented Nov 1, 2022

Oh, it looks like the DoS protection in rust-bitcoin is causing this. Cc @RCasatta

@NicolasDorier
Copy link

NicolasDorier commented Nov 1, 2022

I bumped to 0.15.4, it fixed the issue. (our own build) wrong issue, I panic-commented.

@Kixunil
Copy link
Contributor

Kixunil commented Nov 1, 2022

@NicolasDorier you're replying to electrs issue, I guess you wanted some LND-related issue. :)

@junderw
Copy link

junderw commented Nov 1, 2022

Oh, it looks like the DoS protection in rust-bitcoin is causing this. Cc @RCasatta

@Kixunil I was looking for the issue and had it pinpointed to somewhere in the diff between 0.27.0 and 0.28.0 (which is a huge diff, lol)

One of our servers was on 0.27 and bumping to 0.28 fixed the issue.

Out of curiosity, where was the bug triggered?

@Kixunil
Copy link
Contributor

Kixunil commented Nov 1, 2022

@junderw actually I just realized I got confused since the issue is actually fixed. I explicitly tried deserializing the offending tx and it works fine today. I will publish the test so you can run git-bisect if you want.

Kixunil added a commit to Kixunil/rust-bitcoin that referenced this issue Nov 1, 2022
This transaction broke past versions of `rust-bitcoin` and LND so this
adds a test to avoid reintroducing the problem in the future.

See also romanz/electrs#783
@Kixunil
Copy link
Contributor

Kixunil commented Nov 1, 2022

@junderw here's the test: rust-bitcoin/rust-bitcoin#1359

@romanz
Copy link
Owner

romanz commented Nov 1, 2022

Since the issue should be resolved in bitcoin 0.28.0, please upgrade to electrs 0.9.8+
https://crates.io/crates/electrs/0.9.8/dependencies

@romanz romanz added the p2p label Nov 1, 2022
@romanz
Copy link
Owner

romanz commented Nov 1, 2022

I think that the issue is due to rust-bitcoin/rust-bitcoin#1359 (comment)

IIUC, the failure is due to byte_size being 12000072 = 500003 (len) * 24 (mem::size_of::<$type>):

                let byte_size = (len as usize)
                                    .checked_mul(mem::size_of::<$type>())
                                    .ok_or(self::Error::ParseFailed("Invalid length"))?;
                if byte_size > MAX_VEC_SIZE {
                    return Err(self::Error::OversizedVectorAllocation { requested: byte_size, max: MAX_VEC_SIZE })
                }

and indeed the witness definition has changed between 0.27.1 and 0.28.0:

diff --git a/src/blockdata/transaction.rs b/src/blockdata/transaction.rs
index 4ecac18..445da7d 100644
--- a/src/blockdata/transaction.rs
+++ b/src/blockdata/transaction.rs
@@ -196,7 +198,7 @@ pub struct TxIn {
     /// Encodable/Decodable, as it is (de)serialized at the end of the full
     /// Transaction. It *is* (de)serialized with the rest of the TxIn in other
     /// (de)serialization routines.
-    pub witness: Vec<Vec<u8>>
+    pub witness: Witness
 }

@romanz
Copy link
Owner

romanz commented Nov 1, 2022

IIUC, the fix was introduced into rust-bitcoin in rust-bitcoin/rust-bitcoin@2fd0125 and released in rust-bitcoin 0.28.0-rc1.

@romanz
Copy link
Owner

romanz commented Nov 1, 2022

Closing for now, since the fix is available in electrs 0.9.8+

@sanket1729
Copy link

sanket1729 commented Nov 1, 2022

Reposting my comment here. 0.28 does not completely fix the issue. rust-bitcoin/rust-bitcoin#1359 (comment)

Upon more investigation, this issue is not completely fixed in 0.28. The PR that changes Witness only accidentally fixes it for 0.28 while decoding things with a large Witness. rust-bitcoin/rust-bitcoin#997 lists other things that are still possible to crash with 0.28. This is cleanly fixed in 0.28 with rust-bitcoin/rust-bitcoin#1023.

This is fixed cleanly to the best of my knowledge in 0.29.

@Kixunil
Copy link
Contributor

Kixunil commented Nov 3, 2022

Yeah electrs uses 0.29.2 in master but that is not yet released. @romanz is there anything blocking a release?

@romanz
Copy link
Owner

romanz commented Nov 3, 2022

Will prepare a new release today :)

@romanz romanz mentioned this issue Nov 3, 2022
sanket1729 added a commit to rust-bitcoin/rust-bitcoin that referenced this issue Nov 4, 2022
d6ca7e4 Add a test parsing transaction with a huge witness (Martin Habovstiak)

Pull request description:

  This transaction broke past versions of `rust-bitcoin` and LND so this adds a test to avoid reintroducing the problem in the future.

  See also romanz/electrs#783

  I'm publishing this immediately for research purposes. I can clean it up later if required (low on time rn) or we may even not merge it if there is a better test.

ACKs for top commit:
  tcharding:
    ACK d6ca7e4

Tree-SHA512: cfa9f5c82be0885a82bddb0c15f3177e05feedb369a931e58bf48d90b26eab85bc501d84d17927fa72b42ad4f1016e6042ad318662403271c738eaa91cee7748
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2p
Projects
None yet
Development

No branches or pull requests