diff --git a/.gitignore b/.gitignore index b8edfb6..5083c09 100644 --- a/.gitignore +++ b/.gitignore @@ -332,3 +332,4 @@ __pycache__/ *.so *bore.zip *bore_bin +.venv/ diff --git a/minichain/state.py b/minichain/state.py index 13c7c02..60c3cab 100644 --- a/minichain/state.py +++ b/minichain/state.py @@ -42,6 +42,14 @@ def get_account(self, address): } return self.accounts[address] + def _validate_fee(self, fee): + """ + Shared fee validation used by both verify_transaction_logic and + validate_and_apply, so the rule lives in exactly one place. + Uses exact type check to reject bool (since bool is a subclass of int). + """ + return type(fee) is int and fee >= 0 + def verify_transaction_logic(self, tx): if not tx.verify(): logger.error("Error: Invalid signature for tx from %s...", tx.sender[:8]) @@ -51,9 +59,14 @@ def verify_transaction_logic(self, tx): logger.error("Error: Invalid chain_id in tx from %s...", tx.sender[:8]) return False + fee = getattr(tx, 'fee', 0) + if not self._validate_fee(fee): + logger.error("Error: Invalid fee in tx from %s...", tx.sender[:8]) + return False + sender_acc = self.get_account(tx.sender) - total_cost = tx.amount + getattr(tx, 'fee', 0) + total_cost = tx.amount + fee if sender_acc['balance'] < total_cost: logger.warning("Invalid tx %s: insufficient balance", tx.tx_id) return False @@ -93,9 +106,16 @@ def validate_and_apply(self, tx): semantic validation entry points. """ # Semantic validation: amount must be an integer and non-negative - if not isinstance(tx.amount, int) or tx.amount < 0: + if type(tx.amount) is not int or tx.amount < 0: return None - # Further checks can be added here + + fee = getattr(tx, 'fee', 0) + if not self._validate_fee(fee): + return None + + if type(tx.nonce) is not int or tx.nonce < 0: + return None + return self.apply_transaction(tx) def apply_transaction(self, tx): @@ -109,7 +129,7 @@ def apply_transaction(self, tx): sender = self.accounts[tx.sender] total_cost = tx.amount + getattr(tx, 'fee', 0) - + # Deduct funds and increment nonce sender['balance'] -= total_cost sender['nonce'] += 1 @@ -174,7 +194,7 @@ def apply_transaction(self, tx): # Execution & transfers valid: commit state changes atomically self.update_contract_storage(tx.receiver, result["storage"]) - + receiver['balance'] -= total_transferred_out for t in transfers: target_acc = self.get_account(t["to"]) @@ -217,4 +237,4 @@ def update_contract_storage_partial(self, address, updates): def credit_mining_reward(self, miner_address, reward=None): reward = reward if reward is not None else self.DEFAULT_MINING_REWARD account = self.get_account(miner_address) - account['balance'] += reward + account['balance'] += reward \ No newline at end of file diff --git a/tests/test_negative_fee.py b/tests/test_negative_fee.py new file mode 100644 index 0000000..d272bb8 --- /dev/null +++ b/tests/test_negative_fee.py @@ -0,0 +1,136 @@ +import unittest +from nacl.signing import SigningKey +from nacl.encoding import HexEncoder + +from minichain import State, Transaction + + +class TestNegativeFeePrevention(unittest.TestCase): + def setUp(self): + self.state = State() + self.state.chain_id = "minichain-default" + + # Setup Alice with a small balance + self.alice_sk = SigningKey.generate() + self.alice_pk = self.alice_sk.verify_key.encode(encoder=HexEncoder).decode() + self.state.credit_mining_reward(self.alice_pk, 10) + + self.bob_pk = "b" * 64 + + def _make_tx(self, amount, fee, nonce=0): + tx = Transaction( + sender=self.alice_pk, + receiver=self.bob_pk, + amount=amount, + nonce=nonce, + fee=fee, + chain_id="minichain-default", + ) + tx.sign(self.alice_sk) + return tx + + # ------------------------------------------------------------------ + # Core exploit scenario + # ------------------------------------------------------------------ + + def test_negative_fee_is_rejected(self): + """Negative fee MUST be rejected; balance must not be inflated.""" + initial_balance = self.state.get_account(self.alice_pk)["balance"] + + tx = self._make_tx(amount=0, fee=-1000) + receipt = self.state.validate_and_apply(tx) + + self.assertIsNone(receipt, "validate_and_apply should return None for negative fee") + self.assertEqual( + self.state.get_account(self.alice_pk)["balance"], + initial_balance, + "Alice's balance must remain unchanged after a rejected negative-fee tx", + ) + + def test_negative_fee_does_not_change_nonce(self): + """Rejected tx must not increment the sender nonce.""" + initial_nonce = self.state.get_account(self.alice_pk)["nonce"] + tx = self._make_tx(amount=0, fee=-1) + self.state.validate_and_apply(tx) + self.assertEqual( + self.state.get_account(self.alice_pk)["nonce"], + initial_nonce, + "Nonce must not be incremented for rejected transactions", + ) + + def test_large_negative_fee_is_rejected(self): + """Even a very large negative fee (millions of coins) must be rejected.""" + tx = self._make_tx(amount=0, fee=-10_000_000) + receipt = self.state.validate_and_apply(tx) + self.assertIsNone(receipt) + self.assertEqual(self.state.get_account(self.alice_pk)["balance"], 10) + + def test_fee_of_zero_is_accepted(self): + """Fee of exactly 0 is valid and should be accepted.""" + tx = self._make_tx(amount=5, fee=0) + receipt = self.state.validate_and_apply(tx) + self.assertIsNotNone(receipt, "Fee of 0 should be accepted") + self.assertEqual(receipt.status, 1) + + def test_positive_fee_is_accepted(self): + """A normal positive fee should be accepted and correctly deducted.""" + tx = self._make_tx(amount=3, fee=2) + receipt = self.state.validate_and_apply(tx) + self.assertIsNotNone(receipt, "Positive fee should be accepted") + self.assertEqual(receipt.status, 1) + self.assertEqual(self.state.get_account(self.alice_pk)["balance"], 5) + + def test_float_fee_is_rejected(self): + """Float fees (e.g. -0.5, 1.5) must be rejected as non-integer.""" + for bad_fee in [-0.5, 1.5, 0.1]: + with self.subTest(fee=bad_fee): + tx = Transaction( + sender=self.alice_pk, + receiver=self.bob_pk, + amount=0, + nonce=0, + fee=bad_fee, + chain_id="minichain-default", + ) + tx.sign(self.alice_sk) + receipt = self.state.validate_and_apply(tx) + self.assertIsNone(receipt, f"Float fee {bad_fee} should be rejected") + + def test_negative_nonce_is_rejected(self): + """A negative nonce must also be rejected.""" + tx = Transaction( + sender=self.alice_pk, + receiver=self.bob_pk, + amount=0, + nonce=-1, + fee=0, + chain_id="minichain-default", + ) + tx.sign(self.alice_sk) + receipt = self.state.validate_and_apply(tx) + self.assertIsNone(receipt, "Negative nonce should be rejected") + + def test_bool_fee_is_rejected(self): + """bool is a subclass of int in Python; True/False must not pass as fee.""" + for bad_fee in [True, False]: + with self.subTest(fee=bad_fee): + tx = self._make_tx(amount=0, fee=bad_fee) + receipt = self.state.validate_and_apply(tx) + self.assertIsNone(receipt, f"bool fee {bad_fee} should be rejected") + + def test_apply_transaction_directly_rejects_negative_fee(self): + """Even when validate_and_apply is bypassed, apply_transaction's + underlying verify_transaction_logic must reject negative fees.""" + initial_balance = self.state.get_account(self.alice_pk)["balance"] + tx = self._make_tx(amount=0, fee=-1000) + receipt = self.state.apply_transaction(tx) + self.assertIsNone(receipt, "apply_transaction must reject negative fee directly") + self.assertEqual( + self.state.get_account(self.alice_pk)["balance"], + initial_balance, + "Balance must not inflate when apply_transaction is called directly", + ) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file