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.

Sign up for GitHub

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

Jump to bottom

Fix: PVE-001 Signature Malleability #40

Merged
merged 2 commits into from
Oct 20, 2022
Merged

Fix: PVE-001 Signature Malleability #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4c1d46
Fix: PVE-001 Signature Malleability
x64x48 Oct 19, 2022
a78e746
Fix: PVE-001-3 Signature Malleability
NAOS-Finance-Public Oct 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 2 additions & 25 deletions contracts/protocol/core/UniqueIdentity.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,7 @@ contract UniqueIdentity is BaseUpgradeablePausable, IUniqueIdentity {
bytes32 ethSignedMessage = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", h));
address recovered = tryRecover(ethSignedMessage, signature);
require(hasRole(SIGNER_ROLE, recovered), "Invalid signer");
_;
}

modifier incrementNonce(address account) {
nonces[account] += 1;
_;
}
Expand Down Expand Up @@ -84,7 +81,7 @@ contract UniqueIdentity is BaseUpgradeablePausable, IUniqueIdentity {
uint256 id,
uint256 expiresAt,
bytes calldata signature
) public override onlySigner(_msgSender(), id, expiresAt, signature) incrementNonce(_msgSender()) {
) public override onlySigner(_msgSender(), id, expiresAt, signature) {
require(supportedUIDTypes[id] == true, "Token id not supported");
require(expiration[_msgSender()][id] == 0, "Expiration before must be 0");
require(expiresAt > block.timestamp, "Expiration must be bigger than current timestamp");
Expand All @@ -97,7 +94,7 @@ contract UniqueIdentity is BaseUpgradeablePausable, IUniqueIdentity {
uint256 id,
uint256 expiresAt,
bytes calldata signature
) public override onlySigner(account, id, expiresAt, signature) incrementNonce(account) {
) public override onlySigner(account, id, expiresAt, signature) {
require(expiresAt > block.timestamp, "Expiration must be bigger than current time");
require(supportedUIDTypes[id] == true, "Token id not supported");

Expand All @@ -122,16 +119,6 @@ contract UniqueIdentity is BaseUpgradeablePausable, IUniqueIdentity {
expiration[account][id] = expiresAt;
}

function tryRecover(
bytes32 hash,
bytes32 r,
bytes32 vs
) internal pure returns (address) {
bytes32 s = vs & bytes32(0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff);
uint8 v = uint8((uint256(vs) >> 255) + 27);
return tryRecover(hash, v, r, s);
}

function tryRecover(
bytes32 hash,
uint8 v,
Expand Down Expand Up @@ -173,16 +160,6 @@ contract UniqueIdentity is BaseUpgradeablePausable, IUniqueIdentity {
v := byte(0, mload(add(signature, 0x60)))
}
return tryRecover(hash, v, r, s);
} else if (signature.length == 64) {
bytes32 r;
bytes32 vs;
// ecrecover takes the signature parameters, and the only way to get them
// currently is to use assembly.
assembly {
r := mload(add(signature, 0x20))
vs := mload(add(signature, 0x40))
}
return tryRecover(hash, r, vs);
} else {
revert("InvalidSignatureLength");
}
Expand Down
3 changes: 2 additions & 1 deletion contracts/test/TestUniqueIdentity.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ contract TestUniqueIdentity is UniqueIdentity {
uint256 id,
uint256 expiresAt,
bytes memory data
) public onlyAdmin incrementNonce(to) {
) public onlyAdmin {
nonces[to] += 1;
_updateExpiration(to, id, expiresAt);
}
}