Skip to content

fabtoken: audit path never applies configured resource limits, always falls back to defaults - #2207

Merged
AkramBitar merged 1 commit into
mainfrom
fix-2028
Aug 13, 2026
Merged

fabtoken: audit path never applies configured resource limits, always falls back to defaults#2207
AkramBitar merged 1 commit into
mainfrom
fix-2028

Conversation

@Effi-S

@Effi-S Effi-S commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #2028

Summary

The main fabtoken validator applies deployment-configured resource limits (MaxInputs, MaxOutputs, metadata size caps, etc.) to deserialized actions via SetLimits(...) before calling Deserialize(). The audit-path deserializer does not — it always falls back to driver.DefaultResourceLimits() regardless of what limits the deployment actually configured. This is an inconsistency between the validate and audit code paths: an action that the validator would reject for exceeding configured limits could still be processed by the audit path using looser defaults.

Where

Main validator wires limits through before deserializing (token/core/fabtoken/v1/validator/validator.go:32,43):

ia.SetLimits(a.Limits)
...
ta.SetLimits(a.Limits)

The audit path's ActionDeserializer never does this (token/core/fabtoken/v1/audit/auditor.go:34-56):

func (a *ActionDeserializer) DeserializeActions(tr *driver.TokenRequest) ([]*actions.IssueAction, []*actions.TransferAction, error) {
	issues := tr.GetIssues()
	issueActions := make([]*actions.IssueAction, len(issues))
	for i := range issues {
		ia := &actions.IssueAction{}
		if err := ia.Deserialize(issues[i]); err != nil {   // no ia.SetLimits(...) call before this
			return nil, nil, err
		}
		issueActions[i] = ia
	}

	transfers := tr.GetTransfers()
	transferActions := make([]*actions.TransferAction, len(transfers))
	for i := range transfers {
		ta := &actions.TransferAction{}
		if err := ta.Deserialize(transfers[i]); err != nil {  // no ta.SetLimits(...) call before this
			return nil, nil, err
		}
		transferActions[i] = ta
	}

	return issueActions, transferActions, nil
}

Since TransferAction.effectiveLimits()/IssueAction.effectiveLimits() fall back to driver.DefaultResourceLimits() whenever SetLimits was never called, the audit path always enforces the hardcoded defaults, never the deployment's actual configured limits.

Impact

Wherever the audit path is expected to reject the same class of oversized/abusive actions the validator rejects (excess inputs/outputs, oversized metadata), it currently enforces a different (and potentially looser or stricter) policy than the one operators actually configured — a DoS-protection inconsistency between validation and audit.

Suggested fix

Thread the configured driver.ResourceLimits into audit.NewAuditor/ActionDeserializer (the same way validator.go receives and applies a.Limits), and call ia.SetLimits(...)/ta.SetLimits(...) before Deserialize() in DeserializeActions.

Severity

MEDIUM — inconsistent enforcement between validate and audit paths, not itself an authentication/validity bypass.

@Effi-S Effi-S added this to the Q3/26 milestone Aug 12, 2026
@Effi-S Effi-S added bug Something isn't working security auditor fabtoken labels Aug 12, 2026
@Effi-S Effi-S self-assigned this Aug 12, 2026
@Effi-S
Effi-S marked this pull request as ready for review August 12, 2026 15:15
@Effi-S
Effi-S requested a review from AkramBitar August 12, 2026 15:22
@Effi-S
Effi-S force-pushed the fix-2028 branch 2 times, most recently from 5f2f61e to 28bc385 Compare August 12, 2026 16:17

@AkramBitar AkramBitar left a comment

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.

LGTM

Signed-off-by: Effi-S <effi.szt@gmail.com>
@AkramBitar
AkramBitar merged commit df6ef1a into main Aug 13, 2026
152 checks passed
@Effi-S
Effi-S deleted the fix-2028 branch August 13, 2026 09:47
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.

fabtoken: audit path never applies configured resource limits, always falls back to defaults

2 participants