fix: validate price data before BigInt operations in getFeeQuote

[guvenkaya-security-team]

Closes #166
Closes #165

Adds validation for price data in the getFeeQuote fallback calculation to prevent
runtime errors from invalid external API responses.

Vulnerability Details

The getFeeQuote function in src/lib/estimate-fee.ts (lines 73-83) performs BigInt
arithmetic using price data from tokensUsdPricesHttpClient without validation:

const feePriceScaled = BigInt(Math.round(feeAssetPrice.price * USD_SCALE));
const tokenPriceScaled = BigInt(Math.round(tokenAssetPrice.price * USD_SCALE));
// ...
const den = tokenPriceScaled * 10n ** feeDecimals * 10n;
let exactAmountIn = num / den;  // Division by zero when tokenPriceScaled = 0

Invalid price values can cause:

• Division by zero: When price = 0 or rounds to 0 after scaling
• RangeError: When price is NaN or Infinity (cannot convert to BigInt)
• Incorrect calculations: When price is negative

Solution

Implement price validation before BigInt operations:

  const validatePrice = (price: number, assetId: string): void => {
      if (!Number.isFinite(price) || price <= 0) {
          throw new QuoteError(
              `Invalid price for asset ${assetId}: ${price}. ` +
              `Price must be a positive finite number.`
          );
      }
      if (Math.round(price * USD_SCALE) === 0) {
          throw new QuoteError(
              `Price too small for asset ${assetId}: ${price}. ` +
              `Minimum supported price is ${1 / USD_SCALE} USD.`
          );
      }
  };

  validatePrice(feeAssetPrice.price, feeAssetId);
  validatePrice(tokenAssetPrice.price, tokenAssetId);

As well as validate decimals

const MAX_DECIMALS = 18n;

		if (feeDecimals > MAX_DECIMALS) {
			throw new Error(
				`Fee asset decimals (${feeDecimals}) exceeds maximum allowed (${MAX_DECIMALS})`,
			);
		}

		if (tokenDecimals > MAX_DECIMALS) {
			throw new Error(
				`Token asset decimals (${tokenDecimals}) exceeds maximum allowed (${MAX_DECIMALS})`,
			);
		}

		if (feeDecimals < 0n || tokenDecimals < 0n) {
			throw new Error("Decimals cannot be negative");
		}
		

[changeset-bot]

⚠️ No Changeset found

Latest commit: c41031c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Walkthrough

Added runtime validations in the fallback fee-calculation path: a module-local validatePrice enforces finite, positive prices with a minimum threshold; added non-negative checks and a MAX_DECIMALS = 24n upper bound for decimals before performing fixed-point BigInt arithmetic on feeAssetPrice and tokenAssetPrice.

Changes

Cohort / File(s)	Change Summary
Price & decimals validation packages/intents-sdk/src/lib/estimate-fee.ts	Added a non-exported validatePrice(price, fieldName, USD_SCALE) helper that enforces finite, positive prices and a minimum supported price; added MAX_DECIMALS = 24n and non-negative checks for feeDecimals / tokenDecimals; invoke validations for feeAssetPrice.price, tokenAssetPrice.price, and decimals before the fallback fixed-point BigInt arithmetic.

Sequence Diagram(s)

sequenceDiagram
  participant Caller as getFeeQuote caller
  participant Estimate as estimate-fee.ts
  participant Math as FixedPointArithmetic

  Caller->>Estimate: request fallback fee quote
  Note right of Estimate: obtain feeAssetPrice, tokenAssetPrice, feeDecimals, tokenDecimals
  Estimate->>Estimate: validatePrice(feeAssetPrice.price, ...)
  Estimate->>Estimate: validatePrice(tokenAssetPrice.price, ...)
  Estimate->>Estimate: validateDecimals(feeDecimals, tokenDecimals, MAX_DECIMALS)
  alt validations pass
    Estimate->>Math: perform fixed-point BigInt arithmetic
    Math-->>Estimate: computed fee
    Estimate-->>Caller: return fee quote
  else validation fails
    Estimate-->>Caller: throw validation error
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Check validatePrice thresholds and error messages.
• Verify MAX_DECIMALS = 24n and non-negative decimals checks occur before any BigInt exponentiation.
• Confirm all fallback arithmetic paths call the new validations.

Possibly related PRs

• feat(intents-sdk): fallback to exact_out to get fee tokens #106: Adds input validation and MAX_DECIMALS checks to the same fallback fixed-point arithmetic in packages/intents-sdk/src/lib/estimate-fee.ts.

Poem

🐰 I nibble bugs and tidy sums,
I check each price before it runs.
No zero hop, no infinite leaps,
Decimals bounded, safety keeps.
Hooray — fees safe in cozy drums 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: adding price validation before BigInt operations in getFeeQuote, which is the primary objective of the PR.
Description check	✅ Passed	The description is well-related to the changeset, providing context about the vulnerability, proposed solution, and implementation details for both price and decimals validation.
Linked Issues check	✅ Passed	The PR addresses both linked issues (#166 and #165) by implementing price validation (finite, positive, minimum threshold) and decimals validation (MAX_DECIMALS = 24n check and non-negative validation), meeting all stated coding requirements.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the requirements: price and decimals validation in getFeeQuote. No unrelated modifications are present in the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 764dbf8 and 07319d2.

📒 Files selected for processing (1)

• packages/intents-sdk/src/lib/estimate-fee.ts (2 hunks)

🔇 Additional comments (1)

packages/intents-sdk/src/lib/estimate-fee.ts (1)

70-72: Excellent placement of validation calls.

The validation is correctly positioned after the null checks and before the BigInt operations, preventing runtime errors early in the fallback path.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

packages/intents-sdk/src/lib/estimate-fee.ts (2)

81-99: Validate decimals as integers before BigInt conversion.

The assertions correctly enforce MAX_DECIMALS = 24n and non-negative bounds, addressing PR objective #165. However, if feeAssetPrice.decimals or tokenAssetPrice.decimals is a float, NaN, or Infinity, the BigInt() constructor at lines 81-82 will throw a RangeError before your assertions run, resulting in a less informative error message.

Apply this diff to validate decimals before conversion:

 	const tokenPriceScaled = BigInt(
 		Math.round(tokenAssetPrice.price * USD_SCALE),
 	);
+
+	assert(
+		Number.isInteger(feeAssetPrice.decimals) && feeAssetPrice.decimals >= 0,
+		`Fee asset decimals must be a non-negative integer, got: ${feeAssetPrice.decimals}`,
+	);
+	assert(
+		Number.isInteger(tokenAssetPrice.decimals) && tokenAssetPrice.decimals >= 0,
+		`Token asset decimals must be a non-negative integer, got: ${tokenAssetPrice.decimals}`,
+	);
+
 	const feeDecimals = BigInt(feeAssetPrice.decimals);
 	const tokenDecimals = BigInt(tokenAssetPrice.decimals);
 
 	const MAX_DECIMALS = 24n;
 
 	assert(
 		feeDecimals <= MAX_DECIMALS,
 		`Fee asset decimals (${feeDecimals}) exceeds maximum allowed (${MAX_DECIMALS})`,
 	);
 
 	assert(
 		tokenDecimals <= MAX_DECIMALS,
 		`Token asset decimals (${tokenDecimals}) exceeds maximum allowed (${MAX_DECIMALS})`,
 	);
-
-	assert(
-		feeDecimals >= 0n && tokenDecimals >= 0n,
-		"Decimals cannot be negative",
-	);

Note: The diff removes the redundant non-negative check at lines 96-99 since it's now handled before BigInt conversion with more specific error messages.

---

84-84: Informational: PR description mentions MAX_DECIMALS = 18n, but implementation correctly uses 24n.

The PR description states MAX_DECIMALS = 18n, but the implementation correctly uses 24n to support NEAR's 24 decimals (as indicated in past review feedback). Consider updating the PR description to reflect the actual implementation.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff0417b and c41031c.

📒 Files selected for processing (1)

• packages/intents-sdk/src/lib/estimate-fee.ts (3 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: semgrep-cloud-platform/scan
• GitHub Check: semgrep-cloud-platform/scan

🔇 Additional comments (3)

packages/intents-sdk/src/lib/estimate-fee.ts (3)

2-2: LGTM! Using assert for validation aligns with past feedback.

The import of assert from internal-utils is correct and addresses the previous review feedback to use assert() for validation errors.

---

75-76: LGTM! Price validation correctly placed before BigInt operations.

The calls to validatePrice ensure both prices are validated before scaling and BigInt conversion (lines 77-80), preventing the division-by-zero and invalid BigInt conversion issues described in PR objective #166.

---

150-160: LGTM! Price validation logic is correct and addresses all past feedback.

The validatePrice helper correctly:

• Enforces finite, positive prices
• Uses 1/USD_SCALE for the minimum threshold (as suggested in past feedback), ensuring Math.round(price * USD_SCALE) >= 1 and preventing division by zero
• Uses assert (as suggested in past feedback) for consistent error handling

This addresses all requirements from PR objective #166.