zNS- Using a digital signature as a hash preimage
Using the encoded signature (r,s,v) or the hash of the signature to prevent replay or track if signatures have been seen/used is not recommended in general, as it may introduce signature malleability issues, as two different signature params (r,s,v) may be producable that validly sign the same data.\\nThe impact for this codebase, however, is limited, due to the fact that openzeppelins `ECDSA` wrapper library is used which checks for malleable `ECDSA` signatures (high s value). We still decided to keep this as a medium issue to raise awareness, that it is bad practice to rely on the hash of signatures instead of the hash of the actual signed data for checks.\\nIn another instance in zAuction, a global random nonce is used to prevent replay attacks. This is suboptimal and instead, the hash of the signed data (including a nonce) should be used.\\n```\\n  function fulfillDomainBid(\\n  uint256 parentId,\\n  uint256 bidAmount,\\n  uint256 royaltyAmount,\\n  string memory bidIPFSHash,\\n  string memory name,\\n  string memory metadata,\\n  bytes memory signature,\\n  bool lockOnCreation,\\n  address recipient\\n) external {\\n  bytes32 recoveredBidHash = createBid(parentId, bidAmount, bidIPFSHash, name);\\n  address recoveredBidder = recover(recoveredBidHash, signature);\\n  require(recipient == recoveredBidder, "ZNS: bid info doesnt match/exist");\\n  bytes32 hashOfSig = keccak256(abi.encode(signature));\\n  require(approvedBids[hashOfSig] == true, "ZNS: has been fullfilled");\\n  infinity.safeTransferFrom(recoveredBidder, controller, bidAmount);\\n  uint256 id = registrar.registerDomain(parentId, name, controller, recoveredBidder);\\n  registrar.setDomainMetadataUri(id, metadata);\\n  registrar.setDomainRoyaltyAmount(id, royaltyAmount);\\n  registrar.transferFrom(controller, recoveredBidder, id);\\n  if (lockOnCreation) {\\n    registrar.lockDomainMetadataForOwner(id);\\n  }\\n  approvedBids[hashOfSig] = false;\\n  emit DomainBidFulfilled(\\n    metadata,\\n    name,\\n    recoveredBidder,\\n    id,\\n    parentId\\n  );\\n}\\n```\\n\\n```\\nfunction acceptBid(bytes memory signature, uint256 rand, address bidder, uint256 bid, address nftaddress, uint256 tokenid) external {\\n    address recoveredbidder = recover(toEthSignedMessageHash(keccak256(abi.encode(rand, address(this), block.chainid, bid, nftaddress, tokenid))), signature);\\n    require(bidder == recoveredbidder, 'zAuction: incorrect bidder');\\n    require(!randUsed[rand], 'Random nonce already used');\\n    randUsed[rand] = true;\\n```\\n
Consider creating the bid identifier by hashing the concatenation of all bid parameters instead. Ensure to add replay protection https://github.com/ConsenSys/zer0-zns-audit-2021-05/issues/19. Always check for the hash of the signed data instead of the hash of the encoded signature to track whether a signature has been seen before.\\nConsider implementing Ethereum typed structured data hashing and signing according to EIP-712.
```\\n  function fulfillDomainBid(\\n  uint256 parentId,\\n  uint256 bidAmount,\\n  uint256 royaltyAmount,\\n  string memory bidIPFSHash,\\n  string memory name,\\n  string memory metadata,\\n  bytes memory signature,\\n  bool lockOnCreation,\\n  address recipient\\n) external {\\n  bytes32 recoveredBidHash = createBid(parentId, bidAmount, bidIPFSHash, name);\\n  address recoveredBidder = recover(recoveredBidHash, signature);\\n  require(recipient == recoveredBidder, "ZNS: bid info doesnt match/exist");\\n  bytes32 hashOfSig = keccak256(abi.encode(signature));\\n  require(approvedBids[hashOfSig] == true, "ZNS: has been fullfilled");\\n  infinity.safeTransferFrom(recoveredBidder, controller, bidAmount);\\n  uint256 id = registrar.registerDomain(parentId, name, controller, recoveredBidder);\\n  registrar.setDomainMetadataUri(id, metadata);\\n  registrar.setDomainRoyaltyAmount(id, royaltyAmount);\\n  registrar.transferFrom(controller, recoveredBidder, id);\\n  if (lockOnCreation) {\\n    registrar.lockDomainMetadataForOwner(id);\\n  }\\n  approvedBids[hashOfSig] = false;\\n  emit DomainBidFulfilled(\\n    metadata,\\n    name,\\n    recoveredBidder,\\n    id,\\n    parentId\\n  );\\n}\\n```\\n