Guvenkaya Security Insights Series — Sweat Economy

8 months ago 53
BOOK THIS SPACE FOR AD
ARTICLE AD

Guvenkaya

Welcome to our first entry in our Security Insights blog series, brought to you by Guvenkaya, a leading security research firm specializing in the realm of Rust security, Web3 security of Rust-based protocols, and traditional Web2 security frameworks. Through this series, we aim to share relatively short, insightful reviews of our comprehensive security assessments, offering a window into our analytical process and the critical findings that emerge from our in-depth audits.

In this first installment, we’re diving into an innovative project that stands at the intersection of fitness and cryptocurrency: The Sweat Foundation’s Sweat Economy. This project seeks to motivate users towards greater physical activity by converting their steps into SWEAT Tokens, thereby promoting health and wellness alongside providing an accessible entry point into the world of cryptocurrency.

This blog post aims to provide a concise review of our findings, highlighting specific vulnerabilities and sharing our insights on both the challenges and solutions encountered. We believe that sharing these details not only underscores the importance of thorough security assessments in the rapidly evolving landscape of Web3 and smart contracts but also serves as an educational resource for both project teams and security engineers alike. Join us as we navigate through these discoveries and share practical recommendations to elevate security in such vibrant and evolving projects. Let’s get started!

Before we start, consider following us and Sweat Economy on Social Media:

Sweat Economy WebsiteSweat Economy TwitterGuvenkaya WebsiteGuvenkaya Twitter

In our first Security Insights examination our findings, ranging from low-severity issues to informational insights. Each section of our analysis adheres to a consistent format: we highlight the Observation to outline the issue, discuss its Impact to understand the stakes, and delve into the Proposed Solution alongside the Remediation steps taken.

This structure is designed to not only detail the technicalities of each finding but also to impart actionable knowledge, equipping both burgeoning auditors and innovative projects with the insights needed to fortify their defenses in the evolving digital landscape.

We noted that the claim function attempts to utilize the is_locked field in the AccountRecord as a race condition preventative measure. Ideally, this lock should prevent double spending by ensuring that once an account engages in a transaction, it cannot initiate another until the first is completed. However, the lock’s assertion is not performed at the function’s start, potentially nullifying its effectiveness.

Despite this oversight, our testing revealed no instances of race conditions. This is attributed to the design where accruals are cleared prior to executing a cross-contract call, thereby eliminating any balance that could be erroneously claimed again. Our assessments included both batch calls through near_workspaces and direct smart contract interactions, confirming the absence of adverse effects under current conditions.

The relevant Rust code snippet from sweat-claim:claim:contract/src/claim/api.rs demonstrates the absence of an initial lock assertion:

fn claim(&mut self) -> PromiseOrValue<ClaimResultView> {
let account_id = env::predecessor_account_id();
...
let account_data = self.accounts.get_mut(&account_id).expect("Account data is not found");
account_data.is_locked = true;
...
}

To reinforce the security of the claim function and mitigate any future risk associated with race conditions, we recommend implementing an assertion check at the function’s beginning to confirm the is_locked status. This proactive measure will ensure the lock’s intended functionality is fully operational.

It is crucial for project teams to rigorously enforce and verify security measures like race condition locks. Implementing an early assertion check as part of the function’s logic can significantly enhance the robustness of transactional operations, safeguarding against potential vulnerabilities.

Security engineers should prioritize the identification and assessment of race condition vulnerabilities, especially in environments where transactional integrity is paramount. This finding underscores the importance of comprehensive testing, including scenario-based assessments that go beyond conventional testing frameworks. By exploring both theoretical vulnerabilities and their practical implications, security engineers can provide invaluable insights and recommendations, ensuring the highest standards of security for their clients.

In response to our findings, The Sweat Foundation acted swiftly to fortify the security of their claim function against potential race conditions. The remediation was effectively implemented through the addition of a critical assertion at the beginning of the claim function. This update introduces a check to ensure that no other operation is running for the same account, thereby enforcing the lock mechanism’s intended functionality.

let account_data = self.accounts.get_mut(&account_id).expect("Account data is not found");
// add assert preventing double spending to claim method
require!(!account_data.is_locked, "Another operation is running");
account_data.is_locked = true;
let now = now_seconds();

This crucial adjustment exemplifies The Sweat Foundation’s proactive approach to security and their dedication to maintaining the integrity of their platform. By implementing this assertion, they’ve not only addressed the specific vulnerability we identified but also enhanced the overall robustness of their smart contract against similar issues in the future.

We discovered a scenario where the defer batch operation may fail when using the production batch size of 135. This occurs as the function attempts to log each transaction in the batch, potentially exceeding the log size limit and causing a failure. Our tests simulated realistic conditions, using account lengths and step counts to evaluate the system’s limits. Our initial tests, crafted to mirror the complexity of real-world scenarios, hinted at a potential issue. Yet, it was through meticulous communication and questioning, specifically asking about production parameters, that the true extent of this challenge was unveiled. This operation’s Achilles’ heel was not in its code but in its capacity to withstand the demands of production-level traffic.

This issue poses a risk of transaction failures in production, specifically when batches include entries with account IDs of significant length. It highlights a critical area where theoretical testing diverges from practical, real-world application, underscoring the importance of testing under production-like conditions.

This finding shares a critical lesson for aspiring security auditors and project developers: the essence of effective communication. By inquiring about the production vectors, we peeled back the layers of assumed functionality to reveal a vulnerability that could have remained hidden. It’s a testament to the idea that sometimes, the right question is more powerful than the right answer.

sweat_claim:record_batch_for_hold:contract/src/record/api.rs

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));
}

To mitigate this risk and ensure robustness in production, we recommended the implementation of additional testing that accurately reflects the production batch size. This proactive approach aims to identify and rectify potential failures before they impact live operations.

Projects should implement comprehensive testing strategies that mirror real-world conditions as closely as possible. This includes using production batch sizes in tests to uncover potential issues that may not be evident in smaller-scale or theoretical testing environments. Proactively addressing these concerns can significantly reduce the risk of unexpected failures in production.

Security engineers should advocate for and facilitate the creation of test environments that replicate production conditions, including batch sizes, transaction volumes, and account variability. This approach is crucial for identifying potential bottlenecks or failures that could compromise system integrity under peak loads or specific conditions. By extending testing scenarios to cover these aspects, security professionals can provide more accurate assessments and recommendations, ensuring the systems are not only secure but also resilient and reliable under all conditions.

The Sweat Foundation team addressed this issue by incorporating a test that uses the actual production batch size, effectively validating the system’s capacity to handle real-world transaction volumes without exceeding log size limits.

In the process of auditing the Sweat Economy’s smart contract, specifically the token calculation logic, we identified an issue related to the rounding direction of the oracle fee. The current implementation rounds down the oracle fee, which, for transactions involving a sweat_to_mint value of less than 20, results in an oracle fee of 0. This rounding approach, while seemingly minor, could lead to a cumulative financial shortfall for the protocol over time.

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)
}

Given that SWEAT FT operates with 18 decimal places, occurrences where sweat_to_mint is under 20 are rare. Nonetheless, to safeguard the protocol against any form of cumulative loss or potential exploitation, our recommendation is to adjust the rounding approach in favor of the protocol.

We suggest adopting div_ceil for calculating the oracle fee, ensuring that it rounds up instead of down. This minor adjustment can prevent potential revenue loss for the protocol, ensuring that every fraction of a token is accounted for.

This finding highlights the importance of meticulous attention to financial calculations within smart contracts, especially in protocols handling high volumes of transactions. Small optimizations, such as adjusting the rounding direction, can have significant long-term impacts. Projects are advised to review their rounding logic thoroughly to prevent unintended losses or imbalances.

Security engineers should be vigilant for seemingly inconsequential details, like rounding in financial calculations, that could lead to systemic vulnerabilities or financial discrepancies over time. Encouraging a mindset of precision and caution can aid in identifying areas where small changes yield substantial improvements in security and efficiency.

The Sweat Foundation swiftly addressed this issue by implementing div_ceil for the oracle fee calculation, as detailed in commit e208f3a06050637c6477c73e06cd69a241fe5d7d. This fix exemplifies proactive engagement with audit feedback and a commitment to maintaining the highest standards of protocol integrity and fairness.

During the security assessment, a custom implementation was identified in the on_record callback within the smart contract, aimed at restricting access to oracles only. The custom check was used to verify if the signer is an oracle, an approach that, while functional, diverges from best practices in terms of clarity and security. The core issue lies not in the functionality of the check but in its deviation from conventional Rust and NEAR protocol standards.

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");
}
...
}

This code snippet illustrates the custom check in use, but such an approach can potentially lead to confusion and misinterpretation about the function’s accessibility and its intended use.

To enhance clarity and adhere to the NEAR platform’s best practices, we recommended the adoption of the #[private] macro. This change not only simplifies the access control logic but also clearly indicates that the callback is intended for internal use by the contract only, thereby improving the code’s readability and security.

The transition from custom access control checks to using platform-provided macros like #[private] underscores the importance of adhering to established coding conventions and practices. Such adherence enhances code maintainability, security, and clarity. Projects are encouraged to regularly review their codebase for similar opportunities to refine and align their implementations with best practices.

Security engineers should keep an eye out for unconventional implementations that, while achieving the desired functional outcome, might complicate the codebase or introduce unnecessary risks. Recommending the use of platform-specific features or macros that clearly communicate the intention and scope of a function is a valuable practice. It not only streamlines security audits but also aids developers in maintaining a clean and secure codebase.

The Sweat Foundation implemented the #[private] macro, this fix not only rectifies the identified concern but also serves as a testament to the team’s commitment to security best practices and continuous improvement. By embracing such corrective measures, The Sweat Foundation demonstrates proactive and responsible management of their smart contract environment, ensuring that their innovative platform remains secure, transparent, and aligned with industry standards.

As we wrap up our first security insights series on The Sweat Foundation’s Sweat Economy project, we want to thank the Sweat Foundation for taking security seriously. it’s clear that the intersection of fitness and cryptocurrency presents unique challenges and opportunities for security and innovation. The findings from our audit, ranging from missing race condition locks to the nuances of rounding directions in oracle fees, underscore the complexity and depth required in securing smart contracts within the rapidly evolving Web3 landscape.

We hope this exploration has sparked your curiosity and expanded your understanding. We’re eager to hear your thoughts and invite you to share feedback or questions. Together, let’s continue to explore and secure the evolving world of Web3 Security and decentralized applications. We’re already looking forward to our next post and the insights we can share. Stay tuned, and thank you for joining us on this journey!

If you are building a project and would like Guvenkaya to do a security assessment you can:

Request the quote at: Guvenkaya WebsiteReach out to us at: Timur Guvenkaya TelegramReach out to us via email at: contact@guvenkaya.co
Read Entire Article