Skip to content

getRequestAssetId silently drops a numeric assetId #258

Description

@7r15

Summary

getRequestAssetId in electron/qdn-request-values.ts (shared by both the desktop and Android/renderer bridges) returns undefined for any numeric assetId, even though a numeric assetId is the normal case (qdnRequest({ action: 'PAYMENT', assetId: 5 })). It only works when assetId arrives as a numeric string ("5").

Where

// electron/qdn-request-values.ts:125-129
function getRequestAssetId(request: QdnAppRequest) {
  const value = getRequestValue(request, 'assetId');

  return typeof value === 'undefined' || value === null || getString(value) === '' ? undefined : getInteger(value);
}

getString(value) (same file) returns '' for any non-string value, including numbers:

function getString(value: unknown) {
  return typeof value === 'string' ? value.trim() : '';
}

So for value = 5, getString(5) === '' evaluates to true, and the ternary picks the undefined branch before ever reaching getInteger.

Impact

sendCoinForApp (both electron/qdn.ts and src/platform.ts) uses this to guard PAYMENT/SEND_COIN against non-native assets:

const assetId = getRequestAssetId(request);
if (typeof assetId === 'number' && assetId !== NATIVE_ASSET_ID) {
  throw new Error('Use TRANSFER_ASSET for non-native asset transfers.');
}

With the bug, assetId is undefined for any real (numeric) non-native assetId, so typeof assetId === 'number' is always false and the guard never fires — a non-native asset silently falls through to native-asset handling instead of being rejected. It's also used inside isNativeAssetRequest in the same file.

Suggested fix

getOptionalNonNegativeAssetId (added alongside getAccountBalancePath in #254) already resolves this correctly — it only treats the empty-string check as applying to string values, not numbers:

function getOptionalNonNegativeAssetId(request: QdnAppRequest) {
  const value = getRequestValue(request, 'assetId');

  if (
    typeof value === 'undefined' ||
    value === null ||
    (typeof value === 'string' && getString(value) === '')
  ) {
    return undefined;
  }

  const assetId = getInteger(value);

  if (typeof assetId === 'undefined' || assetId < 0) {
    throw new Error('Asset id must be a non-negative safe integer.');
  }

  return assetId;
}

Replacing getRequestAssetId's two call sites (isNativeAssetRequest, sendCoinForApp in both electron/qdn.ts and src/platform.ts) with getOptionalNonNegativeAssetId should fix it without needing a new helper - though getOptionalNonNegativeAssetId also throws on a negative id, so worth double-checking that's an acceptable behavior change at both call sites before swapping.

Notes

Found while auditing asset-related request parsing for #257. Not fixed there deliberately - it's inside PAYMENT/SEND_COIN's native-asset handling, and Qortium doesn't have a native asset defined yet, so that whole code path is provisional and out of scope for that PR.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions