fix(target_chains/ton): audit fixes (#2092)

* use saturating sub

* use saturating sub for ema price

* validate data sources length

* use the right error type

* fix magic number

* add impure to functions that throw

* dont redefine global vars

* use global var directly

* remove unused variables

* remove debugging code

* remove unused variable num_data_sources

* remove unused variable data_sources

Author: cctdaniel

target_chains/ton/contracts/contracts/Pyth.fc

@@ -17,7 +17,7 @@ cell store_price(int price, int conf, int expo, int publish_time) {
     .end_cell();
 }
 
-slice read_and_verify_header(slice data) {
+slice read_and_verify_header(slice data) impure {
     int magic = data~load_uint(32);
     throw_unless(ERROR_INVALID_MAGIC, magic == ACCUMULATOR_MAGIC);
     int major_version = data~load_uint(8);
@@ -39,7 +39,7 @@ slice read_and_verify_header(slice data) {
     slice cs = read_and_verify_proof(root_digest, message, cs);
 
     int message_type = message~load_uint(8);
-    throw_unless(ERROR_INVALID_MESSAGE_TYPE, message_type == 0);  ;; 0 corresponds to PriceFeed
+    throw_unless(ERROR_INVALID_MESSAGE_TYPE, message_type == PRICE_FEED_MESSAGE_TYPE);
 
     int price_id = message~load_uint(256);
     int price = message~load_int(64);
@@ -80,11 +80,6 @@ int get_governance_data_source_index() method_id {
     return governance_data_source_index;
 }
 
-cell get_data_sources() method_id {
-    load_data();
-    return data_sources;
-}
-
 cell get_governance_data_source() method_id {
     load_data();
     return governance_data_source;
@@ -119,7 +114,7 @@ int get_is_valid_data_source(cell data_source) method_id {
     load_data();
     (int price, int conf, int expo, int publish_time) = get_price_unsafe(price_feed_id);
     int current_time = now();
-    throw_if(ERROR_OUTDATED_PRICE, current_time - publish_time > time_period);
+    throw_if(ERROR_OUTDATED_PRICE, max(0, current_time - publish_time) > time_period);
     return (price, conf, expo, publish_time);
 }
 
@@ -137,7 +132,7 @@ int get_is_valid_data_source(cell data_source) method_id {
     load_data();
     (int price, int conf, int expo, int publish_time) = get_ema_price_unsafe(price_feed_id);
     int current_time = now();
-    throw_if(ERROR_OUTDATED_PRICE, current_time - publish_time > time_period);
+    throw_if(ERROR_OUTDATED_PRICE, max(0, current_time - publish_time) > time_period);
     return (price, conf, expo, publish_time);
 }
 
@@ -202,7 +197,7 @@ int parse_pyth_payload_in_wormhole_vm(slice payload) impure {
     int root_digest = parse_pyth_payload_in_wormhole_vm(payload);
 
     repeat(num_updates) {
-        (int price_id, int price, int conf, int expo, int publish_time, int prev_publish_time, int ema_price, int ema_conf, slice new_cs) = read_and_verify_message(cs, root_digest);
+        (int price_id, int price, int conf, int expo, int publish_time, _, int ema_price, int ema_conf, slice new_cs) = read_and_verify_message(cs, root_digest);
         cs = new_cs;
 
         (slice latest_price_info, int found?) = latest_price_feeds.udict_get?(256, price_id);
@@ -240,7 +235,7 @@ int parse_pyth_payload_in_wormhole_vm(slice payload) impure {
     last_executed_governance_sequence = sequence;
 }
 
-(int, int, slice) parse_governance_instruction(slice payload) {
+(int, int, slice) parse_governance_instruction(slice payload) impure {
     int magic = payload~load_uint(32);
     throw_unless(ERROR_INVALID_GOVERNANCE_MAGIC, magic == GOVERNANCE_MAGIC);
 
@@ -334,6 +329,9 @@ int apply_decimal_expo(int value, int expo) {
         new_data_sources~udict_set(256, data_source_key, begin_cell().store_int(true, 1).end_cell().begin_parse());
     }
 
+    ;; Verify that all data in the payload was processed
+    throw_unless(ERROR_INVALID_PAYLOAD_LENGTH, payload.slice_empty?());
+
     is_valid_data_source = new_data_sources;
 }
 

target_chains/ton/contracts/contracts/Wormhole.fc

@@ -14,19 +14,6 @@
     "NULLSWAPIFNOT" ;; If recovery failed, insert null under the top of the stack
     "NULLSWAPIFNOT2";  ;; If recovery failed, insert two more nulls under the top of the stack
 
-;; For troubleshooting purposes
-() dump_guardian_sets(cell keys_dict) impure {
-    int key = -1;
-    do {
-        (key, slice value, int found) = keys_dict.udict_get_next?(32, key);
-        if (found) {
-            ~dump(key);
-            ~dump(value);
-        }
-    } until (~ found);
-}
-
-
 ;; Internal helper methods
 (int, cell, int) parse_guardian_set(slice guardian_set) {
     slice cs = guardian_set~load_ref().begin_parse();
@@ -130,7 +117,7 @@ int governance_action_is_consumed(int hash) method_id {
     load_data();
     ;; Parse VM fields
     int version = in_msg_body~load_uint(8);
-    throw_unless(ERROR_INVALID_VERSION, version == 1);
+    throw_unless(ERROR_INVALID_VERSION, version == WORMHOLE_VM_VERSION);
     int vm_guardian_set_index = in_msg_body~load_uint(32);
     ;; Verify and check if guardian set is valid
     (int expiration_time, cell keys_dict, int key_count) = get_guardian_set_internal(vm_guardian_set_index);
@@ -189,16 +176,16 @@ int governance_action_is_consumed(int hash) method_id {
     );
 }
 
-(int, int, int, cell, int) parse_encoded_upgrade(int current_guardian_set_index, slice payload) impure {
+(int, int, int, cell, int) parse_encoded_upgrade(int guardian_set_index, slice payload) impure {
     int module = payload~load_uint(256);
     throw_unless(ERROR_INVALID_MODULE, module == UPGRADE_MODULE);
 
     int action = payload~load_uint(8);
-    throw_unless(ERROR_INVALID_GOVERNANCE_ACTION, action == 2);
+    throw_unless(ERROR_INVALID_GOVERNANCE_ACTION, action == GUARDIAN_SET_UPGRADE_ACTION);
 
     int chain = payload~load_uint(16);
     int new_guardian_set_index = payload~load_uint(32);
-    throw_unless(ERROR_NEW_GUARDIAN_SET_INDEX_IS_INVALID, new_guardian_set_index == (current_guardian_set_index + 1));
+    throw_unless(ERROR_NEW_GUARDIAN_SET_INDEX_IS_INVALID, new_guardian_set_index == (guardian_set_index + 1));
 
     int guardian_length = payload~load_uint(8);
     throw_unless(ERROR_INVALID_GUARDIAN_SET_KEYS_LENGTH, guardian_length > 0);
@@ -232,10 +219,8 @@ int governance_action_is_consumed(int hash) method_id {
     (int version, int vm_guardian_set_index, int timestamp, int nonce, int emitter_chain_id, int emitter_address, int sequence, int consistency_level, slice payload, int hash) = parse_and_verify_wormhole_vm(in_msg_body);
 
     ;; Verify the emitter chain and address
-    int governance_chain_id = get_governance_chain_id();
     throw_unless(ERROR_INVALID_GOVERNANCE_CHAIN, emitter_chain_id == governance_chain_id);
-    int governance_contract_address = get_governance_contract();
-    throw_unless(ERROR_INVALID_GOVERNANCE_CONTRACT, emitter_address == governance_contract_address);
+    throw_unless(ERROR_INVALID_GOVERNANCE_CONTRACT, emitter_address == governance_contract);
 
     ;; Check if the governance action has already been consumed
     throw_if(ERROR_GOVERNANCE_ACTION_ALREADY_CONSUMED, governance_action_is_consumed(hash));

target_chains/ton/contracts/contracts/common/constants.fc

@@ -5,11 +5,16 @@ const int GOVERNANCE_MODULE = 1;
 const int MAJOR_VERSION = 1;
 const int MINIMUM_ALLOWED_MINOR_VERSION = 0;
 
+const int WORMHOLE_VM_VERSION = 1;
+
 const int GUARDIAN_SET_EXPIRY = 86400; ;; 1 day in seconds
 const int UPGRADE_MODULE = 0x0000000000000000000000000000000000000000000000000000000000436f7265; ;; "Core" (left-padded to 256 bits) in hex
+const int GUARDIAN_SET_UPGRADE_ACTION = 2;
 
 const int WORMHOLE_MERKLE_UPDATE_TYPE = 0;
 
+const int PRICE_FEED_MESSAGE_TYPE = 0;
+
 {-
   The main workchain ID in TON. Currently, TON has two blockchains:
   1. Masterchain: Used for system-level operations and consensus.

target_chains/ton/contracts/contracts/common/errors.fc

@@ -42,6 +42,7 @@ const int ERROR_INVALID_GOVERNANCE_TARGET = 2015;
 const int ERROR_INVALID_GOVERNANCE_MAGIC = 2016;
 const int ERROR_INVALID_GOVERNANCE_MODULE = 2017;
 const int ERROR_INVALID_CODE_HASH = 2018;
+const int ERROR_INVALID_PAYLOAD_LENGTH = 2019;
 
 ;; Common
 const int ERROR_INSUFFICIENT_GAS = 3000;

target_chains/ton/contracts/contracts/common/storage.fc

@@ -8,8 +8,6 @@ global int single_update_fee;
 ;; DataSource struct: (emitter_chain_id: int, emitter_address: int)
 ;; emitter_chain_id is a 16-bit unsigned integer
 ;; emitter_address is a 256-bit unsigned integer
-global cell data_sources; ;; Dictionary of DataSource tuples, keyed by u8
-global int num_data_sources;
 global cell is_valid_data_source; ;; Dictionary of int (0 as false, -1 as true), keyed by DataSource cell_hash
 global int upgrade_code_hash; ;; 256-bit unsigned integer
 
@@ -37,8 +35,6 @@ global int governance_data_source_index; ;; u32
         .end_cell();
 
     cell data_sources_cell = begin_cell()
-        .store_dict(data_sources)
-        .store_uint(num_data_sources, 32)
         .store_dict(is_valid_data_source)
         .end_cell();
 
@@ -78,8 +74,6 @@ global int governance_data_source_index; ;; u32
 
     cell data_sources_cell = ds~load_ref();
     slice data_sources_slice = data_sources_cell.begin_parse();
-    data_sources = data_sources_slice~load_dict();
-    num_data_sources = data_sources_slice~load_uint(32);
     is_valid_data_source = data_sources_slice~load_dict();
 
     cell guardian_set_cell = ds~load_ref();

target_chains/ton/contracts/contracts/tests/PythTest.fc

@@ -77,7 +77,3 @@
 (int) test_get_is_valid_data_source(cell data_source) method_id {
     return get_is_valid_data_source(data_source);
 }
-
-(cell) test_get_data_sources() method_id {
-    return get_data_sources();
-}

target_chains/ton/contracts/tests/PythTest.spec.ts

@@ -974,11 +974,4 @@ describe("PythTest", () => {
     // Verify that the contract has not been upgraded by attempting to call the new method
     await expect(pythTest.getNewFunction()).rejects.toThrow();
   });
-
-  it("should correctly get data sources", async () => {
-    await deployContract();
-
-    const dataSources = await pythTest.getDataSources();
-    expect(dataSources).toEqual(DATA_SOURCES);
-  });
 });

target_chains/ton/contracts/wrappers/BaseWrapper.ts

@@ -79,24 +79,18 @@ export class BaseWrapper implements Contract {
       priceDict.set(BigInt(config.priceFeedId), priceFeedCell);
     }
 
-    // Create a dictionary for data sources
-    const dataSourcesDict = Dictionary.empty(
-      Dictionary.Keys.Uint(8),
-      Dictionary.Values.Cell()
-    );
     // Create a dictionary for valid data sources
     const isValidDataSourceDict = Dictionary.empty(
       Dictionary.Keys.BigUint(256),
       Dictionary.Values.Bool()
     );
 
     if (config.dataSources) {
-      config.dataSources.forEach((source, index) => {
+      config.dataSources.forEach((source) => {
         const sourceCell = beginCell()
           .storeUint(source.emitterChain, 16)
           .storeBuffer(Buffer.from(source.emitterAddress, "hex"))
           .endCell();
-        dataSourcesDict.set(index, sourceCell);
         const cellHash = BigInt("0x" + sourceCell.hash().toString("hex"));
         isValidDataSourceDict.set(cellHash, true);
       });
@@ -110,8 +104,6 @@ export class BaseWrapper implements Contract {
 
     // Group data sources information
     const dataSourcesCell = beginCell()
-      .storeDict(dataSourcesDict)
-      .storeUint(config.dataSources ? config.dataSources.length : 0, 32)
       .storeDict(isValidDataSourceDict)
       .endCell();