Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Potential CheckTxFee Function Fails to Handle nil gasPrice #3409

Closed
0xM3R opened this issue Jan 24, 2025 · 0 comments · Fixed by #3416
Closed

Potential CheckTxFee Function Fails to Handle nil gasPrice #3409

0xM3R opened this issue Jan 24, 2025 · 0 comments · Fixed by #3416
Assignees
Labels
bug Something isn't working

Comments

@0xM3R
Copy link

0xM3R commented Jan 24, 2025

Background

We received a report from an external researcher highlighting an issue with the CheckTxFee function in the ZetaChain node. The report indicates that the function does not handle nil values for the gasPrice parameter, which could result in a runtime panic.

Description

The CheckTxFee function in the rpc/types/utils.go file does not check if the gasPrice parameter is nil. This can lead to a runtime panic when the function attempts to call the Mul method on gasPrice.

Relevant Code:

Impact

This issue can cause the application to crash unexpectedly, resulting in:

  • A poor user experience.
  • Potential data loss.

Steps to Reproduce

  1. Call the CheckTxFee function with a nil value for gasPrice.
  2. Observe the runtime panic when the function attempts to call gasPrice.Mul().

Recommendation

Add a nil check for the gasPrice parameter within the CheckTxFee function to prevent the runtime panic:

if cap == 0 {
    return nil
}
if gasPrice == nil {
    return errors.New("gasPrice is nil")
}

Internal Analysis

Analysis by @0xM3R:
When tracing the function call, it was found that gasPrice is checked against being nil in the call_tx module:

Adding an additional validation check for gasPrice within the CheckTxFee function is recommended as a defensive programming measure. However, we believe the overall impact is negligible since no other state confusion or side effects were observed in the reported scenario.

Request for Further Validation

Per feedback from @lumtis, further investigation is required to:

  • Confirm the absence of other side effects.
  • Validate the negligible impact assessment.
@0xM3R 0xM3R added the bug Something isn't working label Jan 24, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants