Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| roundsPerEpochUint = minRoundModulus | ||
| } | ||
|
|
||
| mp.nrEpochsChanges = int(epochs) |
Check failure
Code scanning / CodeQL
Incorrect conversion between integer types High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 12 days ago
In general, the problem is that a 64‑bit signed integer parsed from untrusted input is being converted to int without ensuring that the value fits in the target type’s range. The safe pattern is to either (a) parse directly to the final type’s bit size, or (b) keep the wider type and avoid down‑casting, or (c) add explicit upper and lower bounds checks before conversion, clamping/rejecting out‑of‑range values.
The best fix here, without changing existing functionality, is to keep parsing epochs as int64 but constrain it to a safe, reasonable range before converting to int. Because we cannot rely on the platform’s int size, we should use math.MaxInt32/math.MinInt32 as conservative limits, or explicitly document and enforce a smaller operational range. Also, negative or zero epochs do not make sense for “fast‑forwarding”, so we should reject non‑positive values. A straightforward solution in epochsFastForward is:
- Import
math(already used elsewhere in this file as per the initial snippet; if it wasn’t present we would add it). - After parsing
epochs, beforemp.nrEpochsChanges = int(epochs), add checks:- If
epochs <= 0, log a warning and return without changing anything (or set to a default). - If
epochs > math.MaxInt32, cap it atmath.MaxInt32or log and return. To strictly avoid silent truncation, it’s better to log and ignore the request or set it to a safe maximum instead of silently clipping.
- If
Given the existing logging pattern and that epochsFastForward is a control operation, the least surprising behavior is to validate epochs and, on invalid values, log and return early without updating mp.nrEpochsChanges or mp.roundsModulus.
Concretely, in epochsFastForward in process/block/metablock.go:
- After parsing
epochsandroundsPerEpoch, add a block that:- Verifies
epochs > 0. - Verifies
epochs <= math.MaxInt32. - If either check fails, log a warning and
return.
- Verifies
- Leave the rest of the logic intact, so that on valid input behavior is unchanged.
| @@ -5,6 +5,7 @@ | ||
| "encoding/hex" | ||
| "errors" | ||
| "fmt" | ||
| "math" | ||
| "math/big" | ||
| "strconv" | ||
| "strings" | ||
| @@ -3011,11 +3012,23 @@ | ||
| epochs, err := strconv.ParseInt(tokens[1], 10, 64) | ||
| if err != nil { | ||
| log.Error("epochfastforward", "epochs could not be parsed", tokens[1]) | ||
| return | ||
| } | ||
|
|
||
| // validate epochs to avoid overflow when converting to int | ||
| if epochs <= 0 { | ||
| log.Warn("epochfastforward invalid epochs value; must be positive", "epochs", epochs) | ||
| return | ||
| } | ||
| if epochs > math.MaxInt32 { | ||
| log.Warn("epochfastforward epochs value too large; ignoring request", "epochs", epochs, "maxAllowed", math.MaxInt32) | ||
| return | ||
| } | ||
|
|
||
| roundsPerEpoch, err := strconv.ParseInt(tokens[2], 10, 64) | ||
| if err != nil { | ||
| log.Error("epochfastforward", "rounds could not be parsed", tokens[2]) | ||
| return | ||
| } | ||
| roundsPerEpochUint := uint64(roundsPerEpoch) | ||
|
|
# Conflicts: # common/common.go # factory/core/coreComponents.go
# Conflicts: # common/interface.go # consensus/round/round.go
…SF-2.0 # Conflicts: # process/block/metablock.go
…SF-2.0 # Conflicts: # common/common.go
4 participants
Reasoning behind the pull request
Proposed changes
Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?