Date of engagement: 22nd January 2024 - 25th January 2024
-
Timur GüvenkayaPrincipal advisor
Engagement Dashboard
Audit results
Guvenkaya conducted a security assessment of the Sweat Economy Defer feature from 22nd January 2024 to 25th January 2024. During this engagement, a total of 4 findings were reported. 3 of the findings were low and 1 was informational severity. All the issues were fixed by the Sweat Foundation team.
Engagement record
- Client
- The Sweat Foundation Ltd
- System
- SWEAT FT and Sweat Claim contracts on NEAR
- Review period
- 22nd January 2024 - 25th January 2024
- Paths in scope
- 3
- Principal advisor
- Timur Güvenkaya
Timeline
- Review start22nd January 2024
- Draft report26th January 2024
- Final report29th January 2024
Findings by severity
Finding status at final report
Remediation complexity
Impact
Likelihood
Advisory and Client
The advisory
Guvenkaya specializes in security for digital assets and financial infrastructure, from smart contracts and protocols to applications, infrastructure, custody and cryptography.
Our team, including the sitting Heads of Security at Gauntlet and Agora, brings experience from Kraken, OpenZeppelin, Binance, ZKsync, Nillion, Invicti and EY. Across the team: 200+ security reviews, $30B+ in volume secured, 13 published cryptography research papers, and credentials including OSCP, OSWE and CISM.
- Every reviewer has a public bio and is named before you sign. We match expertise to your system, with a principal leading scoping and checking findings.
The client
The Sweat Foundation is an organization behind Sweat Economy, an innovative project at the intersection of fitness and crypto. It motivates users to stay active by converting their steps into SWEAT Token. This approach promotes health and fitness and works as an entry point to crypto for many users.
Scope and Review Boundaries
Review depths
- Deep
- Line-by-line review with explicit attack hypotheses, adversarial reasoning about state transitions, and reproduction attempts for each hypothesis that survived reading.
- Supporting
- The code was read for its effect on a reviewed flow. Behaviour outside that effect was not assessed.
- Targeted
- Entry points were exercised or triaged against the flow's security properties rather than read in full.
This is the code we read, and how closely. 3 paths are in scope. Each path links to the file at the reviewed commit.
| Path | Review depth | Scope note |
|---|---|---|
sweat/ | Deep | Sweat FT |
contract/ | Deep | Sweat Claim |
contract/ | Deep | Sweat Claim |
Out of scope
The audit will include, but is not limited to, reviewing the code for security vulnerabilities, coding practices, and architecture. The audit does not include a review of the dependencies, math.rs, and token calculation functionality which uses math.rs.
Assessment Framework
Guvenkaya rates findings by likelihood and impact so remediation can be prioritized by technical risk, business context, and the decision the engagement is meant to support.
Rating chain applied to every finding
- 01 Likelihood Likely, possible or rare
- 02 Impact Severe, moderate or negligible
- 03 Severity Derived from the matrix below
- 04 Remediation complexity Simple, moderate or difficult
- 05 Status Not fixed, fixed, acknowledged or scheduled
Impact
| Impact | What it means here |
|---|---|
| Severe | Exploitation could result in critical loss or compromise, such as full system control, substantial financial loss, or severe reputational damage. |
| Moderate | Exploitation may lead to limited data loss, partial compromise, moderate financial impact, or noticeable degradation of services. |
| Negligible | Exploitation has minimal impact, such as minor data exposure without significant consequences or slight inconvenience without substantial disruption. |
Likelihood
| Likelihood | What it means here |
|---|---|
| Likely | The vulnerability is easily discoverable and not overly complex to exploit. |
| Possible | The vulnerability presents some challenges either in discovery or in the complexity of the attack. |
| Rare | The vulnerability is either very difficult to discover or complex to exploit, or both. |
This matrix provides a nuanced view, taking into account both the ease of discovering a vulnerability and the complexity involved in exploiting it.
| Attack Complexity \ Discovery Ease | Obvious | Concealed | Hidden |
|---|---|---|---|
| Complex | Possible | Rare | Rare |
| Moderate | Likely | Possible | Rare |
| Straightforward | Likely | Possible | Possible |
Severity
| Severity | What it means here |
|---|---|
| Critical | Assigned to vulnerabilities with severe impact and a likely likelihood of exploitation. |
| High | For vulnerabilities with either severe impact but only a possible likelihood, or moderate impact with a likely likelihood. |
| Medium | Used for vulnerabilities with severe impact but a rare likelihood, moderate impact with a possible likelihood, or negligible impact with a likely likelihood. |
| Low | For vulnerabilities with moderate impact and rare likelihood, or negligible impact with a possible likelihood. |
| Informational | The lowest severity rating, typically for vulnerabilities with negligible impact and a rare likelihood of exploitation. |
| Likelihood \ Impact | Severe | Moderate | Negligible |
|---|---|---|---|
| Likely | Critical | High | Medium |
| Possible | High | Medium | Low |
| Rare | Medium | Low | Informational |
Remediation complexity
This measures how difficult it is to remediate a finding once it has been identified.
| Remediation complexity | What it means here |
|---|---|
| Simple | Patches or fixes are readily available and easily implemented. |
| Moderate | Requires some time and resources to remediate, but well within the capabilities of most organizations. |
| Difficult | Remediation requires significant resources, specialized skills, or substantial changes to systems or architecture. |
Finding status
This tracks the client's response to each finding and whether remediation has been completed, accepted, or scheduled.
| Status | What it means here |
|---|---|
| Not Fixed | Indicates that the vulnerability has been identified but no remedial action has been taken yet. This status is crucial for newly discovered vulnerabilities or those awaiting prioritization. |
| Fixed | This status is applied when the vulnerability has been successfully remediated. It implies that appropriate measures, such as patching, configuration changes, or architectural modifications, have been implemented to resolve the issue. |
| Acknowledged | This status is used for vulnerabilities that have been recognized, but for various reasons (such as risk acceptance, cost, or other business decisions), have not been fixed. It indicates that the risk posed by the vulnerability is known and has been consciously accepted. |
| Scheduled | This status indicates that the vulnerability has been acknowledged and a plan is in place to fix it in the future. It signifies that while remediation hasn't yet occurred, the issue has been prioritized and is part of the planned development roadmap. |
Findings Summary
Findings register
| Finding | Impact | Likelihood | Severity | Remediation complexity | Status |
|---|---|---|---|---|---|
| GUV-1: Race Condition Lock Is Not Asserted | Negligible | Possible | Low | Simple | Fixed |
| GUV-2: Defer Batch Might Fail Under Production Batch Size | Negligible | Possible | Low | Simple | Fixed |
| GUV-3: Suboptimal Rounding Direction For An Oracle Fee | Negligible | Possible | Low | Simple | Fixed |
| GUV-4: Usage Of Custom Check Instead Of #[private] In The Callback | Negligible | Rare | Informational | Simple | Fixed |
Detailed Findings
GUV-1: Race Condition Lock Is Not Asserted
Description
We've observed that the claim function uses the 'is_locked' field in the AccountRecord as a race condition lock to prevent issues such as double spending. However, despite the account being locked prior to making a cross-contract call, the lock is not asserted at the beginning of the function, rendering it ineffective.
fn claim(&mut self) -> PromiseOrValue<ClaimResultView> { let account_id = env::predecessor_account_id(); require!( self.is_claim_available(account_id.clone()) == ClaimAvailabilityView::Available, "Claim is not available at the moment" ); let account_data = self.accounts.get_mut(&account_id).expect("Account data is not found"); account_data.is_locked = true;Fortunately, even without enabled lock, no race condition occurred because accruals were cleared before a cross-contract call. Therefore, even if the function is called again, the user doesn't have any accrued balance to claim. This was tested using both a batch call through near_workspaces and through a smart contract.
#[tokio::test]async fn race_condition_exploit() -> anyhow::Result<()> { let mut context = prepare_contract().await?; let alice = context.alice().await?; let manager = context.manager().await?; let alice_steps = 10_000; let alice_initial_balance = context.ft_contract().ft_balance_of(alice.to_near()).call().await?; let target_token_amount = context.ft_contract().formula(U64(0), alice_steps).call().await?.0; let target_fee = target_token_amount * 5 / 100; let target_effective_token_amount = target_token_amount - target_fee; context .ft_contract() .defer_batch(vec![(alice.to_near(), alice_steps)], context.sweat_claim().account()) .with_user(&manager) .call() .await?; let claim_contract_balance = context .ft_contract() .ft_balance_of(context.sweat_claim().account()) .call() .await?; assert_eq!(claim_contract_balance.0, target_effective_token_amount); let alice_deferred_balance = context .sweat_claim() .get_claimable_balance_for_account(alice.to_near()) .call() .await?; assert_eq!(alice_deferred_balance.0, target_effective_token_amount); alice.batch(&context.sweat_claim().account().to_string().parse()?) .call(Function::new("claim").args_json(json!({}))) .call(Function::new("claim").args_json(json!({}))) .transact().await?.into_result()?; let alice_balance = context.ft_contract().ft_balance_of(alice.to_near()).call().await?; let alice_balance_change = alice_balance.0 - alice_initial_balance.0; assert_eq!(alice_balance_change, target_effective_token_amount); Ok(()) }#[tokio::test]async fn race_condition_exploit() -> anyhow::Result<()> { let mut context = prepare_contract().await?; let exploit_contract = context.worker.clone(); let exploit_contract = exploit_contract.dev_deploy(EXPLOIT_CONTRACT).await?; context .ft_contract() .storage_deposit(Some(exploit_contract.id().clone().parse()?), None) .call() .await?; context .ft_contract() .tge_mint(&exploit_contract.id().clone().parse()?, U128(100_000_000)) .call() .await?; let alice = exploit_contract.id().to_string().parse::<AccountId>()?; let manager = context.manager().await?; let alice_steps = 10_000; let alice_initial_balance = context.ft_contract().ft_balance_of(alice.clone()).call().await?; let target_token_amount = context.ft_contract().formula(U64(0), alice_steps).call().await?.0; let target_fee = target_token_amount * 5 / 100; let target_effective_token_amount = target_token_amount - target_fee; context .ft_contract() .defer_batch(vec![(alice.clone(), alice_steps)], context.sweat_claim().account()) .with_user(&manager) .call() .await?; let claim_contract_balance = context .ft_contract() .ft_balance_of(context.sweat_claim().account()) .call() .await?; assert_eq!(claim_contract_balance.0, target_effective_token_amount); let alice_deferred_balance = context .sweat_claim() .get_claimable_balance_for_account(alice.clone()) .call() .await?; assert_eq!(alice_deferred_balance.0, target_effective_token_amount); exploit_contract.call("call_claim").args_json(json!({"addr": context.sweat_claim().account()})).max_gas() .transact().await?.into_result()?; let alice_balance = context.ft_contract().ft_balance_of(alice).call().await?; let alice_balance_change = alice_balance.0 - alice_initial_balance.0; assert_eq!(alice_balance_change, target_effective_token_amount); Ok(())}#[near_bindgen]impl Contract { pub fn call_claim(addr: AccountId) -> Promise { Promise::new(addr.clone()) .function_call( "claim".into(), json!({}).to_string().as_bytes().to_vec(), 0, (ONE_TERRA * 100).into(), ) .and(Promise::new(addr).function_call( "claim".into(), json!({}).to_string().as_bytes().to_vec(), 0, (ONE_TERRA * 100).into(), )) .then( Self::ext(env::current_account_id()) .with_static_gas((ONE_TERRA * 30).into()) .resolve_callback(), )} pub fn resolve_callback(#[callback_result] result: Result<ClaimResultView, PromiseError>) { match result { Ok(result) => { log!("Success: {:#?}", result); } Err(e) => { log!("Error: {:?}", e); } } }}Recommendation
We propose adding an assertion in the beginning of the claim function
Remediation
The Sweat Foundation team has fixed the issue by adding an assertion in the beginning of the claim function in this commit: 4077a6d51baad63a1bbb8bbfc7b34d6e2f70c9c3
GUV-2: Defer Batch Might Fail Under Production Batch Size
Description
We've noticed that under certain conditions, the defer batch may fail with the production batch size. The backend utilizes a batch size of 135 when invoking the defer function in the FT smart contract. After FT makes a cross-contract call to the claim smart contract to record steps, it generates a log by pushing the (account_id, amount) tuples to the amounts field of the RecordData structure, for each batch entry. This pattern carries a risk of exceeding the log size limit, which could cause a transaction failure. Additionally, no tests use the actual production batch size. Therefore, while increasing the batch size might pass the tests, transactions could still fail in practice due to exceeding prepaid gas or the log size limit.
let mut event_data = RecordData { timestamp: now_seconds, amounts: vec![],};for (account_id, amount) in amounts { event_data.amounts.push((account_id.clone(), amount)); let amount = amount.0; let index = balances.len(); total_balance += amount; balances.push(amount); ... emit(EventKind::Record(event_data));We conducted tests with a constant number of 10,000 steps, which emulates a possible number of steps for each entry. We used accounts of varying lengths to determine the minimum account length required for a batch of 135 entries to fail. After testing, we found that the minimum account length is 57 characters, plus 5 characters from the .near suffix. This means if the backend sends a batch of 135 entries, each with an account length of 62 characters, the transaction will fail due to the log size limit.
#[tokio::test]async fn defer_max() -> anyhow::Result<()> { let mut context = prepare_contract().await?; let manager = context.manager().await?; let long_account = format!("{}.near", "a".repeat(57)) .parse::<AccountId>()?; let vec = vec![(long_account, 10000); 135]; context .ft_contract() .defer_batch(vec, context.sweat_claim().account()) .with_user(&manager) .call() .await?; Ok(())}Recommendation
The likelyhood of this issue occurring in production is low, but we still propose to add a test with the production batch size to ensure that the issue does not occur in the future.
Remediation
The Sweat Foundation team has fixed the issue by adding a test with the production batch size in this commit: 09aa633d7dd408fa6ee8a6fee5604d7be8d85c1f
GUV-3: Suboptimal Rounding Direction For An Oracle Fee
Description
During token calculations, it was observed that the oracle fee is rounded down. When the sweat_to_mint value is less than 20, the oracle fee is 0. In all other cases, the oracle fee is rounded down, which results in a cumulative loss for the protocol (even if it is insignificant). As SWEAT FT has 18 decimals, having a sweat_to_mint value of less than 20 is rare. However, it is recommended to always round against the user to prevent cumulative loss and any potential system exploitation.
pub(crate) fn calculate_tokens_amount(&self, steps: u32) -> (u128, u128) { let sweat_to_mint: u128 = self.formula(self.steps_since_tge, steps).0; let trx_oracle_fee: u128 = sweat_to_mint * 5 / 100; let minted_to_user: u128 = sweat_to_mint - trx_oracle_fee; (minted_to_user, trx_oracle_fee)}Recommendation
We propose to use div_ceil to round up the oracle fee.
Remediation
The Sweat Foundation team has fixed the issue by using div_ceil to round up the oracle fee in this commit: e208f3a06050637c6477c73e06cd69a241fe5d7d
GUV-4: Usage Of Custom Check Instead Of #[private] In The Callback
Description
It was observed that the on_record callback uses a custom check to determine if the signer is an oracle to restrict access. However, this callback should not be directly accessed by anyone. Additionally, the signer account isn't utilized anywhere beyond the initial check. To avoid this and restrict the callback to the contract only, the callback should be marked as #[private]. This approach is more conventional and clarifies the intention.
fn on_record(&mut self, receiver_id: AccountId, amount: U128, fee_account_id: AccountId, fee: U128) { if !self.oracles.contains(&env::signer_account_id()) { panic_str("The operation can be only initiated by an oracle"); } if !is_promise_success() { panic_str("Failed to record data in holding account"); } let mut events: Vec<FtMint> = Vec::with_capacity(2); ...}Recommendation
We propose to use #[private] macro to restrict accesss to the callback.
Remediation
The Sweat Foundation team has fixed the issue by using #[private] macro in this commit: 7bbae6072f8b79fe92c9fff1770233bdb30a06e2
The web version and the PDF (19 pages) are generated from the same source. The report as first delivered is kept in the public reports archive.