Fix impersonate vulnerability (#8)

contracts/sources-registry.fc

@@ -24,6 +24,7 @@ const int error::invalid_cell_code = 902;
 const int error::too_much_value = 901;
 const int error::not_enough_value = 900;
 const int error::access_denied = 401;
+const int error::verifier_id_mismatch = 402;
 const int error::unknown_op = 0xffff;
 
 const int min_tons_lower_bound = 65000000; ;; 0.065 TON
@@ -86,25 +87,35 @@ slice calculate_source_item_address(int wc, cell state_init) {
     return ();
   }
   slice sender_address = cs~load_msg_addr();
-
-  int op = in_msg_body~load_uint(32);
-  int query_id = in_msg_body~load_uint(64);
-
+
   var (min_tons, max_tons, admin, verifier_registry, source_item_code) = load_data();
 
-  if (op == op::deploy_source_item) {
-    throw_unless(error::access_denied, equal_slices(sender_address, verifier_registry));
-    throw_if(error::too_much_value, msg_value > max_tons);
-    throw_if(error::not_enough_value, msg_value < min_tons_lower_bound);
-    throw_if(error::not_enough_value, msg_value < min_tons);
-    int verifier_id = in_msg_body~load_uint(256);
-    int verified_code_cell_hash = in_msg_body~load_uint(256);
-    cell source_content = in_msg_body~load_ref();
-    in_msg_body.end_parse();
-    deploy_source_item(verifier_id, verified_code_cell_hash, source_item_code, source_content);
-    return ();
+  if (equal_slices(sender_address, verifier_registry)) {
+    ;; the verifier id authenticated by the verifier registry
+    slice verifier_reg_cell = in_msg_body~load_ref().begin_parse();
+    int verified_verifier_id = verifier_reg_cell~load_uint(256);
+    slice forwarded_message = in_msg_body~load_ref().begin_parse();
+
+    int op = forwarded_message~load_uint(32);
+    int query_id = forwarded_message~load_uint(64);
+
+    if (op == op::deploy_source_item) {
+      throw_if(error::too_much_value, msg_value > max_tons);
+      throw_if(error::not_enough_value, msg_value < min_tons_lower_bound);
+      throw_if(error::not_enough_value, msg_value < min_tons);
+      int verifier_id = forwarded_message~load_uint(256);
+      throw_unless(error::verifier_id_mismatch, verifier_id == verified_verifier_id);
+      int verified_code_cell_hash = forwarded_message~load_uint(256);
+      cell source_content = forwarded_message~load_ref();
+      forwarded_message.end_parse();
+      deploy_source_item(verifier_id, verified_code_cell_hash, source_item_code, source_content);
+      return ();
+    }
   }
 
+  int op = in_msg_body~load_uint(32);
+  int query_id = in_msg_body~load_uint(64);
+
   if (op == op::change_verifier_registry) {
     throw_unless(error::access_denied, equal_slices(sender_address, admin));
     slice new_verifier_registry = in_msg_body~load_msg_addr();

contracts/verifier-registry.fc

@@ -18,7 +18,7 @@ const slice REGISTERED_TEXT = "You were successfully registered as a verifier";
 const slice UPDATED_TEXT    = "You successfully updated verifier data";
 
 const int   EXIT_FEE = 200000000;      ;; 0.20 TON
-const int   STAKE    = 10000000000000; ;; 10000 TON
+const int   STAKE    = 1000000000000; ;; 1000 TON
 
 () save_data() impure inline_ref {
     set_data(begin_cell()
@@ -69,6 +69,7 @@ const int   STAKE    = 10000000000000; ;; 10000 TON
 () update_verifier(int id, slice new_settings, slice sender_address, int balance, int coins_sent) impure inline {
     slice admin = sender_address;
     throw_if(402, new_settings.slice_depth() > 10); ;; should allow for 100 nodes, prevents storage attack
+
     (slice cfg, int ok) = storage::verifiers.udict_get?(256, id);
     if (ok) { ;; exists, check is admin
         admin = cfg~load_msg_addr();
@@ -146,7 +147,13 @@ const int   STAKE    = 10000000000000; ;; 10000 TON
             .store_slice(target_addr)
             .store_coins(0)
             .store_uint(1, 1 + 4 + 4 + 64 + 32 + 1 + 1)
-            .store_ref(payload_to_forward).end_cell(), 64);
+            .store_ref(
+            begin_cell()
+                .store_ref(begin_cell().store_uint(verifier_id, 256).end_cell())  ;; attach the verifier id so downstream contract can ensure message is from the correct source
+                .store_ref(payload_to_forward)            
+            .end_cell()
+        )
+            .end_cell(), 64);
 
     save_data();
 }
@@ -180,7 +187,9 @@ const int   STAKE    = 10000000000000; ;; 10000 TON
         slice new_settings = in_msg_body;
 
         ;; validation
-        in_msg_body~load_uint(8); ;; quorum
+        int quorum = in_msg_body~load_uint(8); 
+        throw_unless(421, quorum > 0);
+
         in_msg_body~load_dict(); ;; pub_key_endpoints
         slice name_ref = in_msg_body~load_ref().begin_parse(); ;; name
         throw_unless(420, string_hash(name_ref) == id);

package.json

@@ -10,14 +10,16 @@
     "test": "node --no-experimental-fetch node_modules/mocha/bin/mocha --exit test/**/*.spec.ts"
   },
   "devDependencies": {
+    "@swc/core": "^1.2.177",
     "@ton-community/blueprint": "^0.10.0",
     "@ton-community/sandbox": "^0.11.0",
     "@ton-community/test-utils": "^0.2.0",
-    "@swc/core": "^1.2.177",
     "@types/chai": "^4.3.0",
+    "@types/inquirer": "^9.0.7",
     "@types/mocha": "^9.0.0",
     "@types/semver": "^7.3.9",
     "axios-request-throttle": "^1.0.0",
+    "bigint-buffer": "^1.1.5",
     "chai": "^4.3.4",
     "dotenv": "^16.0.0",
     "fast-glob": "^3.2.11",
@@ -28,8 +30,7 @@
     "ton-core": "^0.49.0",
     "ton-crypto": "^3.2.0",
     "ts-node": "^10.4.0",
-    "typescript": "^4.5.4",
-    "bigint-buffer": "^1.1.5"
+    "typescript": "^4.5.4"
   },
   "prettier": {
     "printWidth": 100

scripts/set-code-sources-registry.ts

@@ -0,0 +1,38 @@
+import { toNano, Address } from "ton-core";
+import { SourcesRegistry } from "../wrappers/sources-registry";
+import { compile, NetworkProvider } from "@ton-community/blueprint";
+import inquirer from "inquirer";
+
+export async function run(provider: NetworkProvider) {
+  const compiled = await compile("sources-registry");
+
+  const codeCellHash = compiled.hash().toString("base64");
+  const addressToConfirm = Address.parse("[SOURCES_REGISTRY_ADDRESS]");
+
+  const sourcesRegistry = provider.open(SourcesRegistry.createFromAddress(addressToConfirm));
+
+  const { address } = await inquirer.prompt([
+    {
+      type: "input",
+      name: "address",
+      message: `\n\n!!!This will set the code of the Sources Registry contract!!! Proceed with extreme caution!\n\nSources Registry Address: ${addressToConfirm}\nNew Code Cell Hash: ${codeCellHash}\n\nWrite the address of the contract to confirm:`,
+    },
+  ]);
+
+  try {
+    if (!Address.parse(address).equals(addressToConfirm)) {
+      console.log("Address does not match, aborting...");
+      process.exit(1);
+    }
+  } catch (e) {
+    console.log(`Invalid address: ${address}, aborting...`);
+    process.exit(1);
+  }
+
+  await sourcesRegistry.sendChangeCode(provider.sender(), {
+    newCode: compiled,
+    value: toNano("0.05"),
+  });
+
+  console.log("Source Registry set code", sourcesRegistry.address);
+}

test/unit/sources-registry.spec.ts

@@ -41,10 +41,10 @@ describe("Sources", () => {
     sourceRegistryContract = blockchain.openContract(
       SourcesRegistry.create(
         {
-          admin: admin.address, 
+          admin: admin.address,
           verifierRegistryAddress,
-          sourceItemCode
-        }, 
+          sourceItemCode,
+        },
         code
       )
     );
@@ -63,7 +63,6 @@ describe("Sources", () => {
     it("should deploy a source contract item", async () => {
       const send = await sourceRegistryContract.sendDeploySource(
         blockchain.sender(verifierRegistryAddress),
-
         {
           verifierId: specs[0].verifier,
           codeCellHash: specs[0].codeCellHash,
@@ -88,18 +87,23 @@ describe("Sources", () => {
       expect(await parseUrlFromGetSourceItemData(sourceItemContract)).to.equal(specs[0].jsonURL);
     });
 
-    it("disallows a non-verifier reg to deploy a source item", async () => {
-      const notVerifier = await blockchain.treasury("non-verifier");
-      const send = await sourceRegistryContract.sendDeploySource(notVerifier.getSender(), {
-        verifierId: specs[0].verifier,
-        codeCellHash: specs[0].codeCellHash,
-        jsonURL: specs[0].jsonURL,
-        version: 1,
-        value: toNano("0.5"),
-      });
+    it("disallows a spoofed verifier id to set a deploy item", async () => {
+      const send = await sourceRegistryContract.sendDeploySource(
+        blockchain.sender(verifierRegistryAddress),
+
+        {
+          verifierId: specs[0].verifier,
+          codeCellHash: specs[0].codeCellHash,
+          jsonURL: specs[0].jsonURL,
+          version: 1,
+          value: toNano("0.5"),
+        },
+        "spoofedVerifier"
+      );
+
       expect(send.transactions).to.have.transaction({
-        from: notVerifier.address,
-        exitCode: 401,
+        from: verifierRegistryAddress,
+        exitCode: 402,
       });
     });
   });
@@ -459,6 +463,19 @@ describe("Sources", () => {
       });
     });
   });
+
+  describe("No op", () => {
+    it("throws on no ops", async () => {
+      const notVerifier = await blockchain.treasury("non-verifier");
+
+      const send = await sourceRegistryContract.sendNoOp(notVerifier.getSender());
+
+      expect(send.transactions).to.have.transaction({
+        from: notVerifier.address,
+        exitCode: 0xffff,
+      });
+    });
+  });
 });
 
 async function parseUrlFromGetSourceItemData(