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

Reorg race condition that can cause rare crashes #408

Closed
defuse opened this issue Jul 28, 2022 · 2 comments
Closed

Reorg race condition that can cause rare crashes #408

defuse opened this issue Jul 28, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@defuse
Copy link
Collaborator

defuse commented Jul 28, 2022

There's a race condition that can cause a lightwalletd crash when a reorg occurs at just the right time.

getBlockFromRPC first calls getblock to get the block into the block variable:

block := parser.NewBlock()
rest, err := block.ParseFromSlice(blockData)

Then, it makes another call to getblock and puts the block data into the block1 variable:

params[1] = json.RawMessage("1") // JSON with list of txids
result, rpcErr := RawRequest("getblock", params)
if rpcErr != nil {
return nil, errors.Wrap(rpcErr, "error requesting verbose block")
}
var block1 ZcashRpcReplyGetblock1
err = json.Unmarshal(result, &block1)

It then loops over block's transactions, but indexes into block1. If block and block1 are identical blocks, this should be fine. But if block has more transactions than block1, which could occur if there's a reorg at just the right time, it will index out of bounds into block1 and crash.

for i, t := range block.Transactions() {
txid, err := hex.DecodeString(block1.Tx[i])

The data here is coming from zcashd, so this crash can not be triggered intentionally to DoS the server.

@defuse defuse added the bug Something isn't working label Jul 28, 2022
@defuse
Copy link
Collaborator Author

defuse commented Jul 28, 2022

Suggested fix: Make the verbose call to getblock first (to get block1), then fetch the block using getblock <hash> (to get block). Using the hash prevents fetching a different block. The reorg-handling logic elsewhere in lightwalletd will deal with the reorg.

@LarryRuane
Copy link
Collaborator

Closed by #409

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

No branches or pull requests

2 participants