Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Another concurrent submission may have created the store first. | ||
| txStore = a.accountStore.GetTxStore(tx.FromAddress.String()) | ||
| if txStore == nil { | ||
| ctxLogger.Errorw("failed to create tx store", "fromAddress", tx.FromAddress.String(), "error", err) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Copilot Autofix
AI 3 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| // Another concurrent submission may have created the store first. | ||
| txStore = a.accountStore.GetTxStore(tx.FromAddress.String()) | ||
| if txStore == nil { | ||
| ctxLogger.Errorw("failed to create tx store", "fromAddress", tx.FromAddress.String(), "error", err) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
General approach: avoid logging sensitive or user-identifying fields in clear text. Instead, either omit them entirely from logs or log a redacted version (e.g., truncated or hashed), while preserving enough information for debugging. In this case, we should ensure that errors returned from AccountStore.CreateTxStore do not contain the full account address, so when those errors are logged upstream, no sensitive data is exposed.
Best targeted fix without changing functionality: update AccountStore.CreateTxStore in relayer/txm/txstore.go so that the returned error no longer embeds accountAddress. We can still indicate that a TxStore already exists, but we should not include the specific address string. This way, when err is logged in signAndBroadcast (ctxLogger.Errorw("failed to create tx store", "fromAddress", tx.FromAddress.String(), "error", err)), the error field itself will not leak the address. Since the fromAddress is already separately logged in the same statement, if you want to fully satisfy the spirit of the rule you could also consider redacting that, but the CodeQL path specifically flags the flow through the error, so the minimal necessary fix is in CreateTxStore.
Concretely:
- In
relayer/txm/txstore.go, modify thefmt.Errorfcall inCreateTxStoreto remove the%splaceholder and theaccountAddressargument:- From:
fmt.Errorf("TxStore already exists: %s", accountAddress) - To:
fmt.Errorf("TxStore already exists")
- From:
- No new imports or helper methods are needed.
This preserves all behavior except that the error text is slightly less specific and no longer contains the account address string.
| @@ -197,7 +197,7 @@ | ||
| defer c.lock.Unlock() | ||
| _, ok := c.store[accountAddress] | ||
| if ok { | ||
| return nil, fmt.Errorf("TxStore already exists: %s", accountAddress) | ||
| return nil, fmt.Errorf("TxStore already exists") | ||
| } | ||
| store := NewTxStore(initialNonce) | ||
| c.store[accountAddress] = store |
1 participant
No description provided.