(docs): guard unsafe int/uint conversions flagged by gosec #2679

sampras343 · 2025-11-10T18:12:34Z

Summary

This PR fixes several gosec G115 findings across Rekor’s verification and Trillian client code. The core issue was unsafe, direct conversions between uint64 & int64 and vice versa on values coming from checkpoints or log roots.
Even though these values are expected to be small in practice, gosec flags them because of a possibility of converting a large uint64 to int64 can overflow and produce incorrect or negative values.
To make these conversions safe and reusable, a small utility was introduced to perform bounds-checked conversions, and the affected call sites were updated to use it. Additional tests were added to cover the new failure paths.

Issues raised:

gosec rule G115 flags these because:
- uint64 -> int64 can overflow if the value is greater than math.MaxInt64
- int64 -> uint64 can wrap if the value is negative

Below are the HIGH Severity issues flagged by gosec

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/verify/verify.go:71�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    70: 		}
  > 71: 	case oldTreeSize > int64(newSTH.Size):
    72: 		return errors.New("inclusion proof returned a tree size larger than the verified tree size")

Autofix: 

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/verify/verify.go:53�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    52: 		consistencyParams.FirstSize = &oldTreeSize      // Root size at the old, or trusted state.
  > 53: 		consistencyParams.LastSize = int64(newSTH.Size) // Root size at the new state to verify against.
    54: 		consistencyParams.TreeID = &treeID

Autofix: 

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/verify/verify.go:50�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    49: 		}
  > 50: 	case oldTreeSize < int64(newSTH.Size):
    51: 		consistencyParams := tlog.NewGetLogProofParamsWithContext(ctx)

Autofix: 

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/verify/verify.go:46�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    45: 		return errors.New("consistency proofs can not be computed starting from an empty log")
  > 46: 	case oldTreeSize == int64(newSTH.Size):
    47: 		if !bytes.Equal(oldSTH.Hash, newSTH.Hash) {

Autofix: 

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/verify/verify.go:42�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    41: 	oldSTH *util.SignedCheckpoint, newSTH *util.SignedCheckpoint, treeID string) error {
  > 42: 	oldTreeSize := int64(oldSTH.Size)
    43: 	switch {

Autofix: 

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/trillianclient/trillian_client.go:327�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    326: 			LeafHash: hashValue,
  > 327: 			TreeSize: int64(root.TreeSize),
    328: 		})

Autofix: 

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/trillianclient/trillian_client.go:243�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    242: 			LeafIndex: index,
  > 243: 			TreeSize:  int64(root.TreeSize),
    244: 		})

Autofix: 

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/sharding/ranges.go:119�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    118: 		}
  > 119: 		l.inactive[i].TreeLength = int64(root.TreeSize)
    120: 		sthMap[r.TreeID] = root

Autofix: 

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/api/tlog.go:167�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    166: 	hashString := hex.EncodeToString(root.RootHash)
  > 167: 	treeSize := int64(root.TreeSize)
    168: 

Autofix: 

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/api/tlog.go:68�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    67: 	hashString := hex.EncodeToString(root.RootHash)
  > 68: 	treeSize := int64(root.TreeSize)
    69: 

Autofix: 

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/api/entries.go:449�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    448: 	inclusionProof := models.InclusionProof{
  > 449: 		TreeSize:   conv.Pointer(int64(root.TreeSize)),
    450: 		RootHash:   conv.Pointer(hex.EncodeToString(root.RootHash)),

Autofix: 

[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/api/entries.go:130�[0m] - G115 (CWE-190): integer overflow conversion uint64 -> int64 (Confidence: MEDIUM, Severity: HIGH)
    129: 	inclusionProof := models.InclusionProof{
  > 130: 		TreeSize:   conv.Pointer(int64(root.TreeSize)),
    131: 		RootHash:   conv.Pointer(hex.EncodeToString(root.RootHash)),

Autofix: 
[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/verify/verify.go:165�[0m] - G115 (CWE-190): integer overflow conversion int64 -> uint64 (Confidence: MEDIUM, Severity: HIGH)
    164: 	if err := proof.VerifyInclusion(rfc6962.DefaultHasher, uint64(*e.Verification.InclusionProof.LogIndex),
  > 165: 		uint64(*e.Verification.InclusionProof.TreeSize), leafHash, hashes, rootHash); err != nil {
    166: 		return err

Autofix: 
[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/verify/verify.go:164�[0m] - G115 (CWE-190): integer overflow conversion int64 -> uint64 (Confidence: MEDIUM, Severity: HIGH)
    163: 
  > 164: 	if err := proof.VerifyInclusion(rfc6962.DefaultHasher, uint64(*e.Verification.InclusionProof.LogIndex),
    165: 		uint64(*e.Verification.InclusionProof.TreeSize), leafHash, hashes, rootHash); err != nil {

Autofix: 
[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/trillianclient/trillian_client.go:247�[0m] - G115 (CWE-190): integer overflow conversion int64 -> uint64 (Confidence: MEDIUM, Severity: HIGH)
    246: 	if resp != nil && resp.Proof != nil {
  > 247: 		if err := proof.VerifyInclusion(rfc6962.DefaultHasher, uint64(index), root.TreeSize, resp.GetLeaf().MerkleLeafHash, resp.Proof.Hashes, root.RootHash); err != nil {
    248: 			return &Response{

Autofix: 
[�[97;41m/home/sacm/Documents/Sigstore/rekor/pkg/api/api.go:153�[0m] - G115 (CWE-190): integer overflow conversion int64 -> uint64 (Confidence: MEDIUM, Severity: HIGH)
    152: 		}
  > 153: 		cp, err := util.CreateAndSignCheckpoint(ctx, viper.GetString("rekor_server.hostname"), r.TreeID, uint64(r.TreeLength), root.RootHash, r.Signer)
    154: 		if err != nil {

Summary of Changes Made

New utility for safe conversions. This was introduced because even when we checked the bounds before conversion, gosec still flagged them as highly severe issues
- pkg/util/safe_convertors.go
G115 (CWE-190): integer overflow conversion fixes in multiple files
- pkg/api/api.go
- pkg/api/entries.go
- pkg/api/tlog.go
- pkg/sharding/ranges.go
- pkg/trillianclient/trillian_client.go
- pkg/verify/verify.go
Relevant unit tests were also added

Why

It preserves the original logic (we still verify consistency/inclusion the same way).
It makes the code robust if in the future a log/root/checkpoint with an unexpectedly large size appears.
It centralizes conversion logic so future G115s can be fixed in one place.
Additionally, when value is simply too large it uses the appropriate code codes.OutOfRange at relevant areas.

Release Note

NONE

Documentation

NONE

Signed-off-by: Sachin Sampras M <[email protected]>

haydentherapper

I don't think this is necessary. 2^63 is a massive number and realistically there will be no logs with that many entries.

sampras343 · 2025-11-10T21:40:18Z

I don't think this is necessary. 2^63 is a massive number and realistically there will be no logs with that many entries.

@haydentherapper Thanks for the feedback, totally fair point, and I agree that reaching 2^63 entries is practically impossible.

The intent of this change wasn’t to handle a realistic overflow scenario, but to make the code explicitly safe and pass gosec’s G115 check (which flags unchecked uint64 → int64 conversions and vice versa).

Adding the bounds check prevents a potential overflow and helps our security linting CI pass cleanly without having to suppress the warning.

I’m happy to drop or adjust the change if you prefer to handle this via a lint exclusion instead, just wanted to ensure we stay compliant with the static analysis rules.

haydentherapper · 2025-11-11T16:03:43Z

Since we've moved development to sigstore/rekor-tiles, we're trying to minimize unnecessary changes to sigstore/rekor. Could you instead add //nolint comments so that gosec doesn't complain?

sampras343 · 2025-11-11T16:28:50Z

Since we've moved development to sigstore/rekor-tiles, we're trying to minimize unnecessary changes to sigstore/rekor. Could you instead add //nolint comments so that gosec doesn't complain?

Perfect, I will do the same. Thanks

Signed-off-by: Sachin Sampras M <[email protected]>

sampras343 · 2025-11-12T00:52:29Z

@haydentherapper Necessary //nolint comments are added at appropriate places.

haydentherapper

Thanks!

haydentherapper · 2025-11-12T00:56:32Z

pkg/verify/verify.go


 // ProveConsistency verifies consistency between an initial, trusted STH
 // and a second new STH. Callers MUST verify signature on the STHs'.
+// nolint


Can you comment the specific lines rather than the function so the func still linted?

Done @haydentherapper

codecov · 2025-11-12T01:03:51Z

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.15%. Comparing base (488eb97) to head (7b175b6).
⚠️ Report is 573 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/api/api.go	0.00%	1 Missing ⚠️
pkg/api/tlog.go	50.00%	1 Missing ⚠️
pkg/trillianclient/trillian_client.go	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2679       +/-   ##
===========================================
- Coverage   66.46%   26.15%   -40.31%     
===========================================
  Files          92      191       +99     
  Lines        9258    20126    +10868     
===========================================
- Hits         6153     5264      -889     
- Misses       2359    14033    +11674     
- Partials      746      829       +83

Flag	Coverage Δ
e2etests	`49.69% <60.00%> (+2.14%)`	⬆️
unittests	`16.68% <46.66%> (-31.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Sachin Sampras M <[email protected]>

haydentherapper

thanks!

sampras343 added 2 commits November 7, 2025 20:18

(fix): gosec issues for integer overflow conversion

b63bbf5

Signed-off-by: Sachin Sampras M <[email protected]>

(fix): gosec issues for integer overflow conversion in trillian client

3ef1052

Signed-off-by: Sachin Sampras M <[email protected]>

sampras343 requested a review from a team as a code owner November 10, 2025 18:12

sampras343 marked this pull request as draft November 10, 2025 18:13

sampras343 added 2 commits November 10, 2025 18:18

(fix): gosec high severity issues

947d35e

Signed-off-by: Sachin Sampras M <[email protected]>

(docs): add relevant comments

dedebbd

Signed-off-by: Sachin Sampras M <[email protected]>

sampras343 force-pushed the fix/linters-checks branch from ab54b84 to dedebbd Compare November 10, 2025 18:19

sampras343 marked this pull request as ready for review November 10, 2025 18:19

haydentherapper reviewed Nov 10, 2025

View reviewed changes

sampras343 added 2 commits November 12, 2025 00:49

(docs): add nolint gosec comments

3d6a43a

Signed-off-by: Sachin Sampras M <[email protected]>

(docs): lint whitespace fix

2f28ba1

Signed-off-by: Sachin Sampras M <[email protected]>

haydentherapper reviewed Nov 12, 2025

View reviewed changes

(docs): added line wise no lint comments

7b175b6

Signed-off-by: Sachin Sampras M <[email protected]>

sampras343 changed the title ~~(fix): guard unsafe int/uint conversions flagged by gosec~~ (docs): guard unsafe int/uint conversions flagged by gosec Nov 12, 2025

haydentherapper approved these changes Nov 12, 2025

View reviewed changes

haydentherapper enabled auto-merge (squash) November 12, 2025 15:51

haydentherapper merged commit 2d4e985 into sigstore:main Nov 12, 2025
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

(docs): guard unsafe int/uint conversions flagged by gosec #2679

(docs): guard unsafe int/uint conversions flagged by gosec #2679

Uh oh!

sampras343 commented Nov 10, 2025

Uh oh!

haydentherapper left a comment

Uh oh!

sampras343 commented Nov 10, 2025

Uh oh!

haydentherapper commented Nov 11, 2025

Uh oh!

sampras343 commented Nov 11, 2025

Uh oh!

sampras343 commented Nov 12, 2025

Uh oh!

haydentherapper left a comment

Uh oh!

haydentherapper Nov 12, 2025

Uh oh!

sampras343 Nov 12, 2025

Uh oh!

codecov bot commented Nov 12, 2025 •

edited

Loading

Uh oh!

haydentherapper left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

(docs): guard unsafe int/uint conversions flagged by gosec #2679

(docs): guard unsafe int/uint conversions flagged by gosec #2679

Uh oh!

Conversation

sampras343 commented Nov 10, 2025

Summary

Issues raised:

Summary of Changes Made

Why

Release Note

Documentation

Uh oh!

haydentherapper left a comment

Choose a reason for hiding this comment

Uh oh!

sampras343 commented Nov 10, 2025

Uh oh!

haydentherapper commented Nov 11, 2025

Uh oh!

sampras343 commented Nov 11, 2025

Uh oh!

sampras343 commented Nov 12, 2025

Uh oh!

haydentherapper left a comment

Choose a reason for hiding this comment

Uh oh!

haydentherapper Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

sampras343 Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

haydentherapper left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Nov 12, 2025 •

edited

Loading