Merge pull request #519 from graphprotocol/audit/gns-ownership-m01

Audit: [M01] Semantic overloading of approvals for access control

Author: abarmat

contracts/discovery/GNS.sol

@@ -131,7 +131,7 @@ contract GNS is GNSV2Storage, GraphUpgradeable, IGNS, Multicall {
      * @dev Modifier that allows only a subgraph operator to be the caller
      */
     modifier onlySubgraphAuth(uint256 _subgraphID) {
-        require(_isApprovedOrOwner(msg.sender, _subgraphID), "GNS: Must be authorized");
+        require(ownerOf(_subgraphID) == msg.sender, "GNS: Must be authorized");
         _;
     }
 

test/gns.test.ts

@@ -641,7 +641,7 @@ describe('GNS', () => {
             newSubgraph1.subgraphDeploymentID,
             newSubgraph1.versionMetadata,
           )
-        await expect(tx).revertedWith('ERC721: operator query for nonexistent token')
+        await expect(tx).revertedWith('ERC721: owner query for nonexistent token')
       })
 
       it('reject if not the owner', async function () {
@@ -696,7 +696,7 @@ describe('GNS', () => {
             newSubgraph1.versionMetadata,
           )
         // NOTE: deprecate burns the Subgraph NFT, when someone wants to publish a new version it won't find it
-        await expect(tx).revertedWith('ERC721: operator query for nonexistent token')
+        await expect(tx).revertedWith('ERC721: owner query for nonexistent token')
       })
     })
 
@@ -722,13 +722,13 @@ describe('GNS', () => {
             newSubgraph1.versionMetadata,
           )
         // NOTE: deprecate burns the Subgraph NFT, when someone wants to publish a new version it won't find it
-        await expect(tx).revertedWith('ERC721: operator query for nonexistent token')
+        await expect(tx).revertedWith('ERC721: owner query for nonexistent token')
       })
 
       it('reject if the subgraph does not exist', async function () {
         const subgraphID = randomHexBytes(32)
         const tx = gns.connect(me.signer).deprecateSubgraph(subgraphID)
-        await expect(tx).revertedWith('ERC721: operator query for nonexistent token')
+        await expect(tx).revertedWith('ERC721: owner query for nonexistent token')
       })
 
       it('reject deprecate if not the owner', async function () {