Fix/bugbot security fixes

CHANGELOG.md

@@ -1,5 +1,45 @@
 # Changelog
 
+## [1.3.2] - 2025-12-03
+
+### Security Fixes (Bugbot Review)
+
+This release addresses critical security issues identified by Cursor Bugbot code review.
+
+### Fixed
+
+#### C# Library
+- **SorobanHelper: Data truncation vulnerability** - `EncodeBytesAsScVal` and `EncodeStringAsScVal` now validate input length and throw `ArgumentException` for data exceeding 255 bytes, preventing silent data corruption
+- **SorobanRpcClient: XDR boolean decode false positives** - Replaced unsafe `xdrBytes.Any(b => b == 0x01)` heuristic with proper SCVal format parsing using type discriminant validation
+
+#### Rust Smart Contract
+- **Balance comparison logic bug** - `verify_balance_proof` now uses proper numeric comparison via `parse_decimal_to_scaled()` instead of incorrect byte length comparison (`balance_data.len() >= required_amount_data.len()`)
+- **Malformed input vulnerability** - `parse_decimal_to_scaled()` now returns `None` for malformed inputs like "-", ".", or empty bytes instead of `Some(0)`, preventing invalid balance data from being treated as zero
+- **Test algorithm mismatch** - All tests now use `compute_test_hmac()` with proper HMAC-SHA256 (RFC 2104 with ipad/opad) instead of plain SHA256, matching production contract behavior
+
+### Added
+- `SorobanHelper.MaxBytesLength` constant (255) for explicit length limit documentation
+- `parse_decimal_to_scaled()` function in Rust contract for accurate decimal number parsing with `has_digits` validation
+- `compute_test_hmac()` helper in Rust tests for consistent HMAC computation
+- `test_verify_balance_proof_insufficient()` test case for balance < required scenario
+- `test_verify_balance_proof_malformed_input()` test case for malformed inputs like "-", "."
+
+### Changed
+- Balance verification now correctly handles edge cases like "99.0" vs "100.0"
+- XDR boolean decoding now validates SCValType discriminant before extracting value
+
+---
+
+## [1.3.1] - 2025-12-03
+
+### Fixed
+- Added `SorobanHelper` class with SCVal encoding/decoding utilities
+- Fixed balance parsing with `CultureInfo.InvariantCulture` for consistent decimal handling
+- Added `using StellarDotnetSdk.Accounts` for `KeyPair` class access in tests
+- Improved test coverage for Stellar integration
+
+---
+
 ## [1.3.0] - 2025-12-02
 
 ### Major Release: Production-Ready Stellar Integration

ZkpSharp/Integration/Stellar/SorobanHelper.cs

@@ -13,13 +13,27 @@ public static class SorobanHelper
         /// </summary>
         /// <param name="bytes">The bytes to encode.</param>
         /// <returns>Base64-encoded SCVal representation.</returns>
+        /// <summary>
+        /// Maximum length for byte data in simplified SCVal encoding.
+        /// For larger data, use proper XDR encoding with Stellar SDK.
+        /// </summary>
+        public const int MaxBytesLength = 255;
+
         public static string EncodeBytesAsScVal(byte[] bytes)
         {
             if (bytes == null || bytes.Length == 0)
             {
                 throw new ArgumentException("Bytes cannot be null or empty.", nameof(bytes));
             }
 
+            if (bytes.Length > MaxBytesLength)
+            {
+                throw new ArgumentException(
+                    $"Bytes length ({bytes.Length}) exceeds maximum allowed ({MaxBytesLength}). " +
+                    "For larger data, use proper XDR encoding with Stellar SDK.",
+                    nameof(bytes));
+            }
+
             // For SCVal bytes type, we prepend a type indicator and encode
             // Type 14 (0x0E) is SCValType::SCV_BYTES in Soroban
             var scVal = new byte[bytes.Length + 4];
@@ -73,6 +87,14 @@ public static string EncodeStringAsScVal(string value)
 
             var bytes = Encoding.UTF8.GetBytes(value);
 
+            if (bytes.Length > MaxBytesLength)
+            {
+                throw new ArgumentException(
+                    $"String byte length ({bytes.Length}) exceeds maximum allowed ({MaxBytesLength}). " +
+                    "For larger strings, use proper XDR encoding with Stellar SDK.",
+                    nameof(value));
+            }
+
             // Type 14 (0x0E) is also used for strings in Soroban (as bytes)
             var scVal = new byte[bytes.Length + 4];
             scVal[0] = 0x0E; // SCValType::SCV_STRING (represented as bytes)

ZkpSharp/Integration/Stellar/SorobanRpcClient.cs

@@ -229,14 +229,42 @@ private bool DecodeBooleanFromXdr(string xdrBase64)
                 // Decode XDR base64 string
                 var xdrBytes = Convert.FromBase64String(xdrBase64);
 
-                // Simplified parsing - proper XDR decoding requires Soroban SDK
-                // This is a best-effort attempt
-                if (xdrBytes.Length >= 2)
+                // Soroban SCVal boolean format:
+                // - First 4 bytes: type discriminant (0x00000000 for SCV_BOOL)
+                // - Next 4 bytes: value (0x00000000 for false, 0x00000001 for true)
+                // Minimum 8 bytes required for a valid SCVal boolean
+
+                if (xdrBytes.Length < 4)
+                {
+                    return false;
+                }
+
+                // Check for SCVal boolean type discriminant
+                // SCV_BOOL = 0, SCV_TRUE = 1, SCV_FALSE = 0 (in older format)
+                // In XDR big-endian format:
+                // - Type 0 (SCV_BOOL) with value in next bytes
+                // - Or Type 1 (SCV_VOID which we treat as false)
+
+                // Check the type discriminant (first 4 bytes, big-endian)
+                int typeDiscriminant = (xdrBytes[0] << 24) | (xdrBytes[1] << 16) | 
+                                       (xdrBytes[2] << 8) | xdrBytes[3];
+
+                // SCValType::SCV_BOOL = 0
+                if (typeDiscriminant == 0 && xdrBytes.Length >= 8)
+                {
+                    // For SCV_BOOL, the value is in the next 4 bytes
+                    int value = (xdrBytes[4] << 24) | (xdrBytes[5] << 16) | 
+                               (xdrBytes[6] << 8) | xdrBytes[7];
+                    return value != 0;
+                }
+
+                // SCValType::SCV_TRUE = 1 (alternative representation)
+                if (typeDiscriminant == 1)
                 {
-                    // Basic heuristic: look for boolean indicators
-                    return xdrBytes.Any(b => b == 0x01);
+                    return true;
                 }
 
+                // Default to false for unknown formats
                 return false;
             }
             catch (FormatException ex)

ZkpSharp/ZkpSharp.csproj

@@ -4,7 +4,7 @@
     <TargetFramework>net8.0</TargetFramework>
     <ImplicitUsings>enable</ImplicitUsings>
     <Nullable>enable</Nullable>
-    <PackageVersion>1.3.1</PackageVersion>
+    <PackageVersion>1.3.2</PackageVersion>
     <PackageReadmeFile>README.md</PackageReadmeFile>
     <PackageProjectUrl>https://github.com/asagynbaev</PackageProjectUrl>
     <RepositoryUrl>https://github.com/asagynbaev/ZkpSharp</RepositoryUrl>
@@ -13,7 +13,7 @@
     <Authors>Azimbek Sagynbaev</Authors>
     <Description>.NET library for implementing Zero-Knowledge Proofs (ZKP) with production-ready Stellar Soroban integration. Supports age, balance, membership, range, and time condition proofs with on-chain verification.</Description>
     <PackageTags>zkp;zero-knowledge-proof;cryptography;privacy;blockchain;stellar;soroban;smart-contracts;ton;solana;hmac</PackageTags>
-    <PackageReleaseNotes>v1.3.1: Bug fixes - added SorobanHelper class, fixed balance parsing with InvariantCulture, improved test coverage.</PackageReleaseNotes>
+    <PackageReleaseNotes>v1.3.2: Security fixes - SorobanHelper validates max 255 bytes to prevent truncation, XDR boolean decode uses proper SCVal format instead of heuristic, Rust contract uses numeric comparison for balance verification, tests use HMAC-SHA256 matching production.</PackageReleaseNotes>
   </PropertyGroup>
 
   <ItemGroup>

contracts/stellar/contracts/proof-balance/src/lib.rs

@@ -100,8 +100,8 @@ impl ZkpVerifier {
     /// # Arguments
     /// * `env` - The Soroban environment
     /// * `proof` - The proof hash to verify
-    /// * `balance_data` - The balance value as bytes
-    /// * `required_amount_data` - The required amount as bytes
+    /// * `balance_data` - The balance value as bytes (decimal string, e.g., "1000.50")
+    /// * `required_amount_data` - The required amount as bytes (decimal string, e.g., "500.25")
     /// * `salt` - The cryptographic salt
     /// * `hmac_key` - The HMAC secret key
     /// 
@@ -128,9 +128,20 @@ impl ZkpVerifier {
             return false;
         }
 
-        // Additional validation: ensure balance >= required_amount
-        // This is a simplified check - in production, you'd parse the actual numeric values
-        let balance_sufficient = balance_data.len() >= required_amount_data.len();
+        // Parse and compare numeric values
+        let balance = Self::parse_decimal_to_scaled(&balance_data);
+        let required = Self::parse_decimal_to_scaled(&required_amount_data);
+
+        let balance_sufficient = match (balance, required) {
+            (Some(b), Some(r)) => b >= r,
+            _ => {
+                env.events().publish(
+                    (Symbol::new(&env, "error"),),
+                    VerificationError::InvalidInput as u32,
+                );
+                false
+            }
+        };
 
         env.events().publish(
             (Symbol::new(&env, "balance_check"),),
@@ -140,6 +151,78 @@ impl ZkpVerifier {
         proof_valid && balance_sufficient
     }
 
+    /// Parses a decimal string (e.g., "1234.56") to a scaled integer for comparison.
+    /// Returns None if parsing fails or if no digits are present.
+    /// The result is scaled by 10^8 to handle up to 8 decimal places.
+    fn parse_decimal_to_scaled(data: &Bytes) -> Option<i128> {
+        // Return None for empty input
+        if data.len() == 0 {
+            return None;
+        }
+
+        let mut result: i128 = 0;
+        let mut decimal_places: u32 = 0;
+        let mut found_decimal = false;
+        let mut is_negative = false;
+        let mut has_digits = false; // Track if at least one digit was parsed
+
+        for i in 0..data.len() {
+            let byte = data.get(i)?;
+
+            // Handle negative sign (only at the start, before any digits)
+            if byte == b'-' && !has_digits && !found_decimal {
+                is_negative = true;
+                continue;
+            }
+
+            // Handle decimal point
+            if byte == b'.' {
+                if found_decimal {
+                    return None; // Multiple decimal points
+                }
+                found_decimal = true;
+                continue;
+            }
+
+            // Handle digits
+            if byte >= b'0' && byte <= b'9' {
+                has_digits = true;
+                let digit = (byte - b'0') as i128;
+                result = result.checked_mul(10)?.checked_add(digit)?;
+
+                if found_decimal {
+                    decimal_places += 1;
+                    if decimal_places > 8 {
+                        // Too many decimal places, stop processing
+                        break;
+                    }
+                }
+            } else if byte != b' ' {
+                // Invalid character (allow spaces to be ignored)
+                return None;
+            }
+        }
+
+        // Must have at least one digit to be valid
+        if !has_digits {
+            return None;
+        }
+
+        // Scale to 8 decimal places for consistent comparison
+        const SCALE_FACTOR: u32 = 8;
+        let scale_needed = SCALE_FACTOR.saturating_sub(decimal_places);
+
+        for _ in 0..scale_needed {
+            result = result.checked_mul(10)?;
+        }
+
+        if is_negative {
+            result = -result;
+        }
+
+        Some(result)
+    }
+
     /// Batch verification of multiple proofs for efficiency.
     /// 
     /// # Arguments