Expiration-related fixes (#899)

* Don't allow using signatures that expire too far in the future.

This prevents users from being able to execute potentially replayable transactions.

* Modify the expiration ledger formula to be consistent with the lifetime.

Before that the entry would live 1 ledger longer than the number of ledgers that it has been bumped for.

* Make nonce expiration ledger to be at least min expiration ledger.

* Make allowance expiration logic consistent with bumps.

soroban-env-host/src/auth.rs

@@ -1172,12 +1172,32 @@ impl AccountAuthorizationTracker {
         }
         self.need_nonce = false;
         if let Some((nonce, expiration_ledger)) = &self.nonce {
-            if host.with_ledger_info(|li| Ok(li.sequence_number))? > *expiration_ledger {
+            let (ledger_seq, max_entry_expiration) =
+                host.with_ledger_info(|li| Ok((li.sequence_number, li.max_entry_expiration)))?;
+            if ledger_seq > *expiration_ledger {
                 return Err(host.err(
                     ScErrorType::Auth,
                     ScErrorCode::InvalidInput,
                     "signature has expired",
-                    &[],
+                    &[
+                        ledger_seq.try_into_val(host)?,
+                        expiration_ledger.try_into_val(host)?,
+                    ],
+                ));
+            }
+            if *expiration_ledger
+                > ledger_seq
+                    .saturating_sub(1)
+                    .saturating_add(max_entry_expiration)
+            {
+                return Err(host.err(
+                    ScErrorType::Auth,
+                    ScErrorCode::InvalidInput,
+                    "signature expiration is too late",
+                    &[
+                        (ledger_seq + max_entry_expiration).try_into_val(host)?,
+                        expiration_ledger.try_into_val(host)?,
+                    ],
                 ));
             }
             if let Some(addr) = self.address {
@@ -1340,6 +1360,8 @@ impl Host {
             nonce_key_scval.metered_clone(self.budget_ref())?,
             xdr::ContractDataDurability::Temporary,
         );
+        let expiration_ledger = expiration_ledger
+            .max(self.get_min_expiration_ledger(xdr::ContractDataDurability::Temporary)?);
         self.with_mut_storage(|storage| {
             if storage.has(&nonce_key, self.budget_ref())? {
                 return Err(self.err(

soroban-env-host/src/host.rs

@@ -2031,8 +2031,12 @@ impl VmCallerEnv for Host {
             .borrow_mut()
             .touch_key(&key, self.as_budget())?;
 
-        let min_expiration =
-            self.with_ledger_info(|li| Ok(li.sequence_number.saturating_add(min.into())))?;
+        let min_expiration = self.with_ledger_info(|li| {
+            Ok(li
+                .sequence_number
+                .saturating_sub(1)
+                .saturating_add(min.into()))
+        })?;
         self.0.expiration_bumps.borrow_mut().metered_push(
             self,
             LedgerBump {
@@ -2048,8 +2052,12 @@ impl VmCallerEnv for Host {
         _vmcaller: &mut VmCaller<Host>,
         min: U32Val,
     ) -> Result<Void, HostError> {
-        let min_expiration =
-            self.with_ledger_info(|li| Ok(li.sequence_number.saturating_add(min.into())))?;
+        let min_expiration = self.with_ledger_info(|li| {
+            Ok(li
+                .sequence_number
+                .saturating_sub(1)
+                .saturating_add(min.into()))
+        })?;
 
         let contract_id = self.get_current_contract_id_internal()?;
         let key = self.contract_instance_ledger_key(&contract_id)?;

soroban-env-host/src/native_contract/token/allowance.rs

@@ -35,7 +35,11 @@ pub fn write_allowance(
     // validates the expiration and then returns the ledger seq
     // The expiration can be less than ledger seq if clearing an allowance
     let ledger_seq = e.with_ledger_info(|li| {
-        if expiration > li.sequence_number.saturating_add(li.max_entry_expiration) {
+        if expiration
+            > li.sequence_number
+                .saturating_sub(1)
+                .saturating_add(li.max_entry_expiration)
+        {
             Err(err!(
                 e,
                 ContractError::AllowanceError,
@@ -93,7 +97,7 @@ pub fn write_allowance(
                 e.bump_contract_data(
                     key.try_into_val(e)?,
                     StorageType::Temporary,
-                    (expiration - ledger_seq).into(),
+                    (expiration - ledger_seq + 1).into(),
                 )?;
             }
         }

soroban-env-host/src/test/auth.rs

@@ -91,6 +91,12 @@ impl AuthTest {
         // to respect the budget limit.
         host.as_budget().reset_unlimited();
         host.enable_debug();
+
+        host.with_mut_ledger_info(|li| {
+            li.sequence_number = 100;
+            li.max_entry_expiration = 10000;
+        })
+        .unwrap();
         let mut accounts = vec![];
         for _ in 0..signer_cnt {
             accounts.push(generate_keypair());

soroban-env-host/src/test/token.rs

@@ -57,7 +57,7 @@ impl TokenTest {
             base_reserve: 5_000_000,
             min_persistent_entry_expiration: 4096,
             min_temp_entry_expiration: 16,
-            max_entry_expiration: 6312000,
+            max_entry_expiration: 10000,
         });
         Self {
             host,
@@ -1658,14 +1658,68 @@ fn test_auth_rejected_for_incorrect_nonce() {
 
     let args = host_vec![&test.host, user.address(&test.host), 100_i128];
 
-    // Correct call to consume nonce.
+    // Expired nonce
     authorize_single_invocation_with_nonce(
         &test.host,
         &admin,
         &token.address,
         "mint",
         args.clone(),
-        Some((12345, 1000)),
+        Some((12345, 122)),
+    );
+    assert!(test
+        .host
+        .call(
+            token.address.clone().into(),
+            Symbol::try_from_small_str("mint").unwrap(),
+            args.clone().into(),
+        )
+        .is_err());
+
+    // Too early for nonce
+    authorize_single_invocation_with_nonce(
+        &test.host,
+        &admin,
+        &token.address,
+        "mint",
+        args.clone(),
+        Some((12345, 123 + 10000)),
+    );
+    assert!(test
+        .host
+        .call(
+            token.address.clone().into(),
+            Symbol::try_from_small_str("mint").unwrap(),
+            args.clone().into(),
+        )
+        .is_err());
+
+    // Correct call to consume nonce that expires on the current ledger.
+    authorize_single_invocation_with_nonce(
+        &test.host,
+        &admin,
+        &token.address,
+        "mint",
+        args.clone(),
+        Some((12345, 123)),
+    );
+    test.host
+        .call(
+            token.address.clone().into(),
+            Symbol::try_from_small_str("mint").unwrap(),
+            args.clone().into(),
+        )
+        .unwrap();
+
+    // Correct call to consume nonce that expires at the maximum possible entry
+    // lifetime.
+    authorize_single_invocation_with_nonce(
+        &test.host,
+        &admin,
+        &token.address,
+        "mint",
+        args.clone(),
+        Some((123456, 123 + 10000 - 1)),
     );
     test.host
         .call(