Skip to content

Commit

Permalink
Security: Move the Verack response after the version check
Browse files Browse the repository at this point in the history
We should do as many local checks as possible, before sending further
messages.
  • Loading branch information
teor2345 committed May 16, 2021
1 parent 1f25d84 commit 66abfbe
Showing 1 changed file with 12 additions and 12 deletions.
24 changes: 12 additions & 12 deletions zebra-network/src/peer/handshake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,18 +493,6 @@ pub async fn negotiate_version(
Err(HandshakeError::NonceReuse)?;
}

peer_conn.send(Message::Verack).await?;

let remote_msg = peer_conn
.next()
.await
.ok_or(HandshakeError::ConnectionClosed)??;
if let Message::Verack = remote_msg {
debug!("got verack from remote peer");
} else {
Err(HandshakeError::UnexpectedMessage(Box::new(remote_msg)))?;
}

// XXX in zcashd remote peer can only send one version message and
// we would disconnect here if it received a second one. Is it even possible
// for that to happen to us here?
Expand Down Expand Up @@ -532,6 +520,18 @@ pub async fn negotiate_version(
Err(HandshakeError::ObsoleteVersion(remote_version))?;
}

peer_conn.send(Message::Verack).await?;

let remote_msg = peer_conn
.next()
.await
.ok_or(HandshakeError::ConnectionClosed)??;
if let Message::Verack = remote_msg {
debug!("got verack from remote peer");
} else {
Err(HandshakeError::UnexpectedMessage(Box::new(remote_msg)))?;
}

Ok((remote_version, remote_services, remote_canonical_addr))
}

Expand Down

0 comments on commit 66abfbe

Please sign in to comment.