Skip to content

security: enforce strict input validation and payload bounds#1706

Open
SurbhiAgarwal1 wants to merge 10 commits into
LFDT-Panurus:mainfrom
SurbhiAgarwal1:security/tokens-validation
Open

security: enforce strict input validation and payload bounds#1706
SurbhiAgarwal1 wants to merge 10 commits into
LFDT-Panurus:mainfrom
SurbhiAgarwal1:security/tokens-validation

Conversation

@SurbhiAgarwal1

Copy link
Copy Markdown
Contributor

Description

This PR implements strict input validation and payload bounds within the tokens.Service.Append method to protect the system against resource exhaustion and potential Denial of Service (DoS) attacks from malformed or oversized finality data.

Changes

  • ValidationConfig: Introduced a configurable validation layer with safe default limits:
    • MaxTokenPayloadSize: 1MB (covers TokenOnLedger and TokenOnLedgerMetadata)
    • MaxTokenOutputsPerTx: 1000 outputs
    • MaxBulkDeleteSize: 10,000 IDs
    • MaxWalletIDSize: 1024 bytes
    • MaxOwnerRawSize: 2048 bytes
    • MaxIssuerRawSize: 2048 bytes
  • AppendRequest: Refactored the internal data flow to use a request-based structure for validated processing.
  • Strict Validation Logic: Implemented validateAppendRequest which performs in-memory checks before any database transactions or storage operations are initiated.
  • Enhanced Testing: Added a comprehensive test suite in token/services/tokens/validation_test.go covering edge cases, limit violations, and structural validation.

Verification Results

  • Unit Tests: All tests in token/services/tokens passed successfully.
  • Linting: Code is clean according to golangci-lint (v2) standards, including nlreturn and testifylint compliance.

Fixes: #1608

Signed-off-by: SurbhiAgarwal1 <agarwalsurbhi1807@gmail.com>
@SurbhiAgarwal1 SurbhiAgarwal1 force-pushed the security/tokens-validation branch from 5cae97f to 8c74cea Compare May 14, 2026 06:11
…k during initialization

Signed-off-by: SurbhiAgarwal1 <agarwalsurbhi1807@gmail.com>
Signed-off-by: SurbhiAgarwal1 <agarwalsurbhi1807@gmail.com>
@adecaro adecaro added this to the Q3/26 milestone Jun 2, 2026
@AkramBitar AkramBitar requested a review from HayimShaul June 9, 2026 07:40
Signed-off-by: SurbhiAgarwal <agarwalsurbhi1807@gmail.com>
Comment thread token/services/tokens/tokens.go Outdated
Comment thread plan.md Outdated
Comment thread token/config.go Outdated
Comment thread token/services/tokens/tokens.go Outdated
Signed-off-by: SurbhiAgarwal1 <agarwalsurbhi1807@gmail.com>
Signed-off-by: SurbhiAgarwal1 <agarwalsurbhi1807@gmail.com>
Comment thread token/services/tokens/tokens.go Outdated
Signed-off-by: SurbhiAgarwal1 <agarwalsurbhi1807@gmail.com>
@HayimShaul

Copy link
Copy Markdown
Contributor

Looks good to me.
@adecaro do you want to have another quick look?

@HayimShaul HayimShaul requested a review from adecaro June 23, 2026 11:01
@mergify

mergify Bot commented Jun 23, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request


// Validate protocol version
if tr.Version == 0 {
return nil, nil, driver.ErrInvalidVersion

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version 0 is now accepted, but it used to be rejected. Why this is removed? This makes validation weaker, which is the opposite of what this PR is for, and it's not related to payload size limits.

Comment thread token/services/tokens/tokens.go Outdated
Storage: storage,
RequestsCache: requestsCache,
Config: ValidationConfig{
MaxTokenPayloadSize: 2 * 1024 * 1024,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defaults duplicated in 4 places

  1. token/driver/validator.go:
    const (
    MaxTokenPayloadSize = 2 * 1024 * 1024
    MaxTokenOutputsPerTx = 1000
    ...
    )

  2. token/config.go — GetValidationConfig:
    config := driver.ValidationConfig{
    MaxTokenPayloadSize: 2 * 1024 * 1024,
    MaxTokenOutputsPerTx: 1000,
    ...
    }

  3. token/core/common/validator.go — NewValidator:
    ValidationConfig: driver.ValidationConfig{
    MaxTokenPayloadSize: 2 * 1024 * 1024,
    MaxTokenOutputsPerTx: 1000,
    ...
    },

  4. token/services/tokens/tokens.go — NewService:
    Config: ValidationConfig{
    MaxTokenPayloadSize: 2 * 1024 * 1024,
    MaxTokenOutputsPerTx: 1000,
    ...
    },

One option to fix this is to define once in token/driver/validator.go, reuse everywhere:

// DefaultValidationConfig holds the default resource limits.
var DefaultValidationConfig = ValidationConfig{
MaxTokenPayloadSize: 2 * 1024 * 1024,
MaxTokenOutputsPerTx: 1000,
MaxBulkDeleteSize: 10000,
MaxWalletIDSize: 1024,
MaxOwnerRawSize: 256 * 1024,
MaxIssuerRawSize: 256 * 1024,
MaxTokenRequestSize: 2 * 1024 * 1024,
MaxActionCount: 1000,
}

if err != nil {
return errors.Wrapf(err, "failed to serialize output at index %d", i)
}
if v.ValidationConfig.MaxTokenPayloadSize > 0 && len(raw) > v.ValidationConfig.MaxTokenPayloadSize {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A limit of 0 means two opposite things:

  • common/validator.go: 0 = no limit (checks are guarded with if MaxX > 0)
  • tokens.go validateAppendRequest: 0 = reject everything (no guard, so len(req.Tokens) > 0 always fails)

So an operator who sets maxTokenOutputsPerTx: 0 expecting "unlimited" would instead block every append.

I think we need to fix this and add the same > 0 guard in tokens.go so 0 means "unlimited" in both places.

@AkramBitar

Copy link
Copy Markdown
Contributor

Hello @SurbhiAgarwal1

I would like to thank you a lot for your work on that PR. I appreciate it a lot.
I added few new comments.
Could your please let me know if you have the time to look at these comments and rebase the PR (have some conflicts)?

Thanks a lot,
Akram

Address PR feedback on validation configs

Signed-off-by: Surbhi Agarwal <SurbhiAgarwal1@users.noreply.github.com>
@SurbhiAgarwal1 SurbhiAgarwal1 force-pushed the security/tokens-validation branch 2 times, most recently from 5cf7641 to 0601d51 Compare July 14, 2026 18:31
…-validation

Signed-off-by: Surbhi Agarwal <SurbhiAgarwal1@users.noreply.github.com>
Signed-off-by: Surbhi Agarwal <SurbhiAgarwal1@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enforce Strict Input Validation and Payload Bounds on Token Store Writes [MED]

4 participants