diff --git a/ethereum/contracts/Messages.sol b/ethereum/contracts/Messages.sol index c41a730c0..de1866309 100644 --- a/ethereum/contracts/Messages.sol +++ b/ethereum/contracts/Messages.sol @@ -108,7 +108,12 @@ contract Messages is Getters { vm.version = encodedVM.toUint8(index); index += 1; - require(vm.version == 1, "VM version incompatible"); + // SECURITY: Note that currently the VM.version is not part of the hash + // and for reasons described below it cannot be made part of the hash. + // This means that this field's integrity is not protected and cannot be trusted. + // This is not a problem today since there is only one accepted version, but it + // could be a problem if we wanted to allow other versions in the future. + require(vm.version == 1, "VM version incompatible"); vm.guardianSetIndex = encodedVM.toUint32(index); index += 4; @@ -129,7 +134,13 @@ contract Messages is Getters { index += 1; } - // Hash the body + /* + Hash the body + + SECURITY: Do not change the way the hash of a VM is computed! + Changing it could result into two different hashes for the same observation. + But xDapps rely on the hash of an observation for replay protection. + */ bytes memory body = encodedVM.slice(index, encodedVM.length - index); vm.hash = keccak256(abi.encodePacked(keccak256(body))); diff --git a/node/pkg/vaa/structs.go b/node/pkg/vaa/structs.go index 13f9fdd9f..b16cd6477 100644 --- a/node/pkg/vaa/structs.go +++ b/node/pkg/vaa/structs.go @@ -434,6 +434,10 @@ func (v *VAA) HexDigest() string { return hex.EncodeToString(v.SigningMsg().Bytes()) } +/* +SECURITY: Do not change this code! Changing it could result in two different hashes for +the same observation. But xDapps rely on the hash of an observation for replay protection. +*/ func (v *VAA) serializeBody() []byte { buf := new(bytes.Buffer) MustWrite(buf, binary.BigEndian, uint32(v.Timestamp.Unix()))