No Stellar public-key format or checksum validation exists anywhere in the DTOs — malformed funder/recipient addresses fall through toScVal's heuristic instead of being rejected at the API boundary

[chonilius]

Overview

Every DTO field representing a Stellar public key — funderAddress on FundEscrowDto, DepositDto, and the inline FundBountyDto/FundMilestoneDto; recipientAddress on ReleaseEscrowDto, SplitRecipientDto, and AssignRewardDto — is validated with nothing more than @IsString():

// src/escrow/dto/fund-escrow.dto.ts:18-20
@ApiProperty({ description: 'Stellar public key of the funding sponsor' })
@IsString()
funderAddress: string;

// src/escrow/dto/release-escrow.dto.ts:4-7
@ApiProperty({ description: 'Stellar public key of the recipient' })
@IsString()
recipientAddress: string;

A real Stellar public key ("StrKey") is a specific, checkable format: it starts with G, is exactly 56 characters, is valid base32, and encodes a version byte plus a CRC16 checksum over the payload — the @stellar/stellar-sdk package already depended on by this codebase exposes StrKey.isValidEd25519PublicKey(string) to validate exactly this, for free, with no new dependency needed. None of these DTOs use it, or any other format check — @IsString() accepts "", "not-an-address", a 55-character near-miss, or a syntactically-plausible-but-checksum-invalid string with equal enthusiasm.

Trace what happens to a garbage address that gets past validation. It reaches SorobanClientService.toScVal's heuristic:

// src/escrow/soroban-client.service.ts:167-176
private toScVal(value: unknown) {
  if (typeof value === 'string' && value.length >= 32 && /^[A-Z0-9]+$/.test(value)) {
    try { return new Address(value).toScVal(); }
    catch { return nativeToScVal(value, { type: 'string' }); }   // <- silent fallback, not a rejection
  }
  // ...
}

An invalid-but-address-shaped string (56 uppercase alphanumeric characters that fail the SDK's checksum) throws inside new Address(value), which is caught and silently downgraded to a generic string encoding — not surfaced as a validation error anywhere. An invalid-and-not-address-shaped string (wrong length, lowercase, whatever) never even reaches the Address constructor — it goes straight to the generic string fallback. Either way, nothing in this pipeline ever produces a clean 400 Bad Request telling the caller "this isn't a valid Stellar address." Whatever eventually happens next depends entirely on how the deployed Soroban contract's Rust code handles being given a malformed Address argument — likely a confusing simulation failure deep inside soroban.invoke (per the companion "no BytesN<32> encoding" issue, this whole path is untested against a real contract today), or, in the worst case, an amount genuinely getting "released" toward an address that was never checked to be a real, spendable Stellar account, silently consuming the escrow's LOCKED funds with nothing to show for it.

This is explicitly not the same gap as the already-closed "amount fields accept unbounded string input" issue in this repo — that issue hardened IsMoneyAmount/isSupportedEscrowAsset for numeric/asset fields specifically and doesn't mention address fields anywhere. It's also distinct from the already-open "Bounty payout to a contributor with no linked Stellar address silently releases to an empty-string recipient" issue — that issue is about a missing address (the ?? '' fallback when stellarAddress is null); this issue is about an address that was provided but was never checked to be syntactically or cryptographically valid in the first place, whether it arrives via the empty-string path or a directly client-supplied funderAddress/recipientAddress on the raw escrow/pool endpoints.

Requirements

• Add a reusable @IsStellarAddress() class-validator decorator (mirroring the existing IsMoneyAmount/IsSupportedEscrowAsset pattern in src/common/validators/money.validator.ts, perhaps in a sibling stellar-address.validator.ts) backed by StrKey.isValidEd25519PublicKey from @stellar/stellar-sdk.
• Apply it to every funderAddress/recipientAddress field currently typed as a bare @IsString(): FundEscrowDto.funderAddress, DepositDto.funderAddress, ReleaseEscrowDto.recipientAddress, SplitRecipientDto.recipientAddress, AssignRewardDto.recipientAddress, and the inline FundBountyDto.funderAddress/FundMilestoneDto.funderAddress in the bounties/milestones controllers.
• Ensure the empty-string fallback from the companion "no linked Stellar address" issue is also caught by this same validator once that issue's fix routes an address through these DTOs/service methods — an empty string should fail IsStellarAddress() too, giving that issue's fix a second, independent layer of protection rather than relying solely on whatever check that issue's own fix adds.
• Add tests: a syntactically-invalid string, a checksum-invalid-but-right-length string, and an empty string are all rejected at the DTO layer with a 400, before ever reaching EscrowService/SorobanClientService.

Acceptance Criteria

• A reusable Stellar-address validator exists and is backed by the SDK's own StrKey checksum validation, not a hand-rolled regex.
• Every DTO field listed above uses it.
• A malformed address (wrong length, wrong prefix, invalid checksum, or empty) is rejected with a 400 at the API boundary, proven by a test that feeds each case directly to the HTTP layer (not just unit-testing the validator function in isolation, matching the rigor of the existing amount-validation fix's own acceptance criteria).
• SorobanClientService.toScVal's silent try/catch fallback for an unparseable address (:172-175) is no longer the only thing standing between a malformed address and a Soroban call — by the time execution reaches this function, the address has already been validated.

Additional Notes

Precise references: src/escrow/dto/fund-escrow.dto.ts:18-20, src/escrow/dto/release-escrow.dto.ts:4-7, src/escrow/dto/split-release.dto.ts:14-17, src/maintenance-pool/dto/create-pool.dto.ts and the inline DepositDto/AssignRewardDto in src/maintenance-pool/maintenance-pool.controller.ts:9-27, the inline FundBountyDto in src/bounties/bounties.controller.ts:10-13, the inline FundMilestoneDto/ResolveIssueDto in src/milestones/milestones.controller.ts:8-20 — every one of these currently uses bare @IsString() for a field that's supposed to be a Stellar public key. src/escrow/soroban-client.service.ts:167-176 (toScVal's silent fallback, the downstream consequence). src/common/validators/money.validator.ts (the existing, already-closed amount-validation fix this issue's requested decorator should sit alongside, following the same IsXyz()/isValidXyz() naming and structure).

Test/reproduction plan:

const cases = ['', 'not-an-address', 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA', /* right length, bad checksum */];
for (const bad of cases) {
  await request(app).post('/escrow/fund')
    .send({ amount: '10.0000000', asset: 'USDC', funderAddress: bad, bountyId })
    .expect(400);
}
const validCases = [Keypair.random().publicKey()];
for (const good of validCases) {
  await request(app).post('/escrow/fund').send({ ...validPayload, funderAddress: good }).expect(201);
}

Cross-references: distinct from, and complementary to, the closed "amount fields accept unbounded string input" issue (same class of gap — DTO-layer validation missing for a financially-critical field type — applied to the address surface that issue didn't cover) and the open "Bounty payout to a contributor with no linked Stellar address silently releases to an empty-string recipient" issue (that issue is about a missing address specifically in the merge-and-release flow; this issue is the general-purpose format/checksum validation that should exist regardless of how an invalid address value arrives, including but not limited to that empty-string path). Also relevant to the companion "no BytesN<32> encoding" issue, since a validated, guaranteed-well-formed address is one less variable to account for once that encoding work is undertaken.

[ghzhost]

I have implemented the fix and opened PR #66 with full test coverage and validation across all Stellar address DTOs and EscrowService methods.

[kosisochukwu1234]

Hello, I’ve gone through the requirements and I’m confident I can implement a proper solution that matches the acceptance criteria. Kindly assign this issue to me.

[oluwarantimini]

The API DTOs are not actually validating that Stellar address fields contain valid Stellar public keys. Every funderAddress and recipientAddress field is protected only by @IsString(), which verifies the JavaScript type but says nothing about the address format.

A Stellar StrKey public key has a specific encoded structure, including the expected prefix, length, base32 representation, version byte, and checksum. The project already depends on @stellar/stellar-sdk, which provides StrKey.isValidEd25519PublicKey() specifically for this validation, so there is no need to introduce another dependency.

With the current validation, values such as an empty string, arbitrary text, incorrectly sized keys, or checksum-invalid StrKeys pass DTO validation and reach deeper application logic before failing—or potentially being stored or processed.

I have experience reviewing API validation boundaries where blockchain-specific identifiers need stronger validation than generic string checks. These DTOs should validate public-key fields with the SDK's StrKey validator (via a custom class-validator constraint or equivalent) so malformed addresses are rejected at the API boundary with a clear validation error.

[grantfox-oss]

🦊 GrantFox — @oluwarantimini has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #60)
2. Your PR will be reviewed by the MergeFi maintainers

Good luck! Track your progress on GrantFox.