Skip to content

fix issues - #36

Open
hundredark wants to merge 2 commits into
mainfrom
security-fix/261023
Open

hundredark wants to merge 2 commits into
mainfrom
security-fix/261023

Conversation

@hundredark

Copy link
Copy Markdown
Contributor

Handle invalid user request memos without panicking.
Restrict asset icon fetching to prevent SSRF and resource exhaustion.
Isolate asset deployment failures so processing can continue and retry later.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Deployment retries can recreate an already-confirmed mint after notification failure, blocking recovery and repeatedly consuming transaction fees.

1 open issue
What changed in this PR

This PR hardens Solana request handling and asset icon fetching, and changes asset deployment failures from process-stopping errors to logged retries.

Changes:

  • Return errors for invalid user app IDs and rent lookup failures.
  • Restrict icon downloads to an approved HTTPS host, with timeout, size, and image-dimension limits.
  • Extract per-asset deployment handling so failures are logged and processing continues.
File Description
solana/​solana.go Return mint rent lookup errors instead of panicking.
solana/​request.go Reject invalid user app IDs without panicking.
solana/​observer.go Isolate deployment errors and retry through the observer loop.
solana/​icon.go Restrict icon origins and bound download and decoding resources.

Review effort: Balanced

Comment thread solana/observer.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Missing-account handling blocks fresh mint deployments, and priority-fee RPC errors still bypass deployment isolation and terminate the process.

2 open issues
1 resolved since last review

Review effort: Balanced

Comment thread apps/solana/rpc.go
Comment on lines +103 to +104
func (c *Client) RPCGetConfirmedAsset(ctx context.Context, address string) (*Asset, error) {
return c.rpcGetAsset(ctx, address, rpc.CommitmentConfirmed)
Comment thread solana/observer.go
}

func (node *Node) deployExternalAsset(ctx context.Context, a *store.ExternalAsset) error {
id, tx, asset, err := node.CreateMintTransaction(ctx, a.AssetId)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants