Sweat EconomyJars Refactor Security Review

Before a user can create a deposit to a jar, the amount is validated against minimum and maximum caps.

jars:assert_cap:contract/src/product/model/v1.rs

     pub(crate) fn assert_cap(&self, amount: TokenAmount) {
             if self.cap.min > amount || amount > self.cap.max {
                env::panic_str(&format!(
                  "Total amount is out of product bounds: [{}..{}]",
                  self.cap.min, self.cap.max
                 ));
              }
           }

After discussion with the team, we discovered that the minimum cap in production is 1 SWEAT. This minimum deposit amount is too low, allowing an attacker to create numerous deposits for any user. This can trigger a gas limit failure in methods like withdraw_all, claim_total, and restake_all, preventing users from performing these operations.

The attack requires approximately $30 in SWEAT and 4,500 deposits. The vulnerability affects all batch operations because the system iterates over all products—even if a user has other products with higher minimum deposits, operations like restake_all, claim_total, and withdraw_all will fail due to the product with excessive deposits.

Recommendation

Consider raising the minimum deposit amount to make the attack economically unviable.

Remediation - Fixed

The Sweat Foundation team has fixed the issue by throttling requests on the backend side, setting keys for all products, add monitoring for jars' creation and stop issuing tickets for suspicious users

The restake method is missing an is_pending_withdraw lock on jars. This lock prevents race conditions before the callback arrives after a cross-contract call. When a jar has is_pending_withdraw set to true, the operation is either skipped or the smart contract returns an error.

jars:assert_not_locked:contract/src/assert.rs

     pub(crate) fn assert_not_locked(jar: &Jar) {
             require!(!jar.is_pending_withdraw, "Another operation on this Jar is in
     progress");
           }

This issue could lead to a scenario where a user's deposit is recorded with a timestamp from the past inside of the cache instead of the actual update time. As a result, interest may begin accumulating earlier than intended.

Possible Scenario:

User deregisters their account on the token contract Calls claim_total, which sets the cache to current timestamp When the callback arrives, it fails due to the unregistered account and rolls back the cache to the previous timestamp while retaining the new deposit.

Recommendation

Consider implementing the is_pending_withdraw check in the restake method.

Remediation - Fixed

The Sweat Foundation team has fixed the issue by adding the race condition lock in this commit: 1b0c87287cd553c7c210acafbfc3c1d3bd63e131

Score-based products are premium products that require a signature. However, during product creation through the register_product method, the public key requirement is not enforced. This allows users to register score-based products without a public key, enabling deposits without any signature verification.

jars:register_product:contract/src/product/api.rs

      fn register_product(&mut self, command: ProductDto) {
              self.assert_manager();
              assert_one_yocto();

assert!(self.products.get(&command.id).is_none(), "Product already exists");

                let product: Product = command.into();

product.assert_fee_amount();

self.products.insert(&product.id, &product);

                emit(EventKind::RegisterProduct(product));
            }

Recommendation

Consider enforcing public key validation in register_product when registering score-based products.

Remediation - Fixed

The Sweat Foundation team has fixed the issue by asseting the public key existence on score based products in this commit:

ba110ce01f9a104e7d533b24bafe57956976f2a9

In the restake_all method's withdrawal fee calculation, the withdrawal fee is rounded down. This rounding behavior results in a cumulative loss of withdrawal fees for the protocol, though the impact is minimal.

jars:restake_all:contract/src/restake/api/api.rs

      if withdrawal_amount > 0 {
                 let withdrawal_fee = total_fee * withdrawal_amount /
     total_mature_balance;
                   request.withdrawal = WithdrawalDto {
                      amount: withdrawal_amount,
                      fee: withdrawal_fee,
                   };

self.transfer_remainder(request, event)

Recommendation

To prevent cumulative losses and potential system exploitation, fees should always be rounded against the user. This can be achieved by using div_ceil to round up the withdrawal_fee.

Remediation - Fixed

The Sweat Foundation team has fixed the issue by utilizing div_ceil in this commit: 9bd95fc264d2dd286082f7d704270accef54dcca

After a recent refactor, the gas measurements for the restake_all operation have become outdated. This could lead to incorrect assumptions about gas consumption.

jars:measure_restake_all:integration-tests/src/measure/restake_all.rs

async fn measure_restake_all() -> Result<()> { set_integration_logs_enabled(false);

              let product = RegisterProductCommand::Locked5Minutes60000Percents;
              let mut context = prepare_contract(None, [product]).await?;
              let alice = context.alice().await?;

              for _ in 0..200 {
                add_jar(&context, &alice, product, 10_000).await?;
              }

context.fast_forward_minutes(6).await?;

context.sweat_jar().claim_total(None).with_user(&alice).await?;

              let gas = context
                .sweat_jar()
                .restake_all(product.get().id, None)
                .with_user(&alice)
                .result()
                .await?
                .total_gas_burnt;
              dbg!(pretty_gas_string(gas));

             // 1 jar - 6 TGas 225 GGas total: 6225437862976
             // 100 jars - 50 TGas 709 GGas total: 50709431315947
             // 200 jars - 86 TGas 607 GGas total: 86607517267105...}

Recommendation

Consider re-running the measurements and updating comments

Remediation - Fixed

The Sweat Foundation team has fixed the issue by re-running the measurements.

The code contains a redundant call to get_or_create_account_mut.

jars:measure_restake_all:integration-tests/src/measure/restake_all.rs

      let account = self.get_or_create_account_mut(&account_id);

                if signature.is_some() {
                   account.nonce += 1;
                }

                if matches!(product.terms, Terms::ScoreBased(_)) {
                   account.try_set_timezone(ticket.timezone);
                }

               let account = self.get_or_create_account_mut(&account_id);

account.deposit(product_id, amount, None);

emit(Deposit((product_id.clone(), amount.into())));

Recommendation

Remove the second get_or_create_account_mut call since the account reference is already available.

Remediation - Fixed

The Sweat Foundation team has fixed the issue by removing the redundant call in this commit: d409a301b5900353245d68f113aac27925b52a07