Skip to content

Commit 43e3be1

Browse files
arimxyerclaude
andcommitted
fix: Add bounds checks and lint fixes for TOTP code
- Add bounds check for key.Period() conversion (max 300 seconds) - Add bounds check for cred.TOTPPeriod before conversion - Fix errcheck: properly ignore resp.Body.Close() error - Fix staticcheck: refactor base32 validation for clarity - Addresses CodeQL security warnings for uint64->int conversions Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
1 parent f7da3ab commit 43e3be1

1 file changed

Lines changed: 18 additions & 8 deletions

File tree

internal/vault/totp.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func CheckTimeSync() TimeSyncResult {
4545
result.Error = err
4646
return result
4747
}
48-
defer resp.Body.Close()
48+
defer func() { _ = resp.Body.Close() }()
4949

5050
result.Checked = true
5151

@@ -142,11 +142,17 @@ func ParseTOTPURI(uri string) (*TOTPConfig, error) {
142142
return nil, fmt.Errorf("unsupported OTP type: %s (only totp is supported)", key.Type())
143143
}
144144

145+
// Safely convert period with bounds check (TOTP periods are typically 30s)
146+
keyPeriod := key.Period()
147+
if keyPeriod > 300 { // Max 5 minutes, reasonable upper bound
148+
keyPeriod = 30 // Default to standard period
149+
}
150+
145151
config := &TOTPConfig{
146152
Secret: key.Secret(),
147153
Issuer: key.Issuer(),
148154
Account: key.AccountName(),
149-
Period: int(key.Period()),
155+
Period: int(keyPeriod), //nolint:gosec // bounds checked above
150156
Digits: key.Digits().Length(),
151157
}
152158

@@ -180,9 +186,12 @@ func ValidateTOTPSecret(secret string) error {
180186
return fmt.Errorf("TOTP secret cannot be empty")
181187
}
182188

183-
// Check for valid base32 characters (A-Z, 2-7)
189+
// Check for valid base32 characters (A-Z, 2-7, =)
184190
for _, c := range secret {
185-
if !((c >= 'A' && c <= 'Z') || (c >= '2' && c <= '7') || c == '=') {
191+
isUpperAlpha := c >= 'A' && c <= 'Z'
192+
isValidDigit := c >= '2' && c <= '7'
193+
isPadding := c == '='
194+
if !isUpperAlpha && !isValidDigit && !isPadding {
186195
return fmt.Errorf("invalid base32 character in TOTP secret: %c", c)
187196
}
188197
}
@@ -218,9 +227,9 @@ func GenerateTOTPCode(cred *Credential) (string, int, error) {
218227
digits = otp.DigitsEight
219228
}
220229

221-
// Determine period
230+
// Determine period with bounds check (TOTP periods are typically 30s, max 5 min)
222231
period := uint(30)
223-
if cred.TOTPPeriod > 0 {
232+
if cred.TOTPPeriod > 0 && cred.TOTPPeriod <= 300 {
224233
period = uint(cred.TOTPPeriod)
225234
}
226235

@@ -235,9 +244,10 @@ func GenerateTOTPCode(cred *Credential) (string, int, error) {
235244
return "", 0, fmt.Errorf("failed to generate TOTP code: %w", err)
236245
}
237246

238-
// Calculate remaining validity
247+
// Calculate remaining validity (period is bounds-checked above, safe to convert)
239248
epoch := now.Unix()
240-
remaining := int(period) - int(epoch%int64(period))
249+
periodInt := int(period) //nolint:gosec // bounds checked above (max 300)
250+
remaining := periodInt - int(epoch%int64(periodInt))
241251

242252
return code, remaining, nil
243253
}

0 commit comments

Comments
 (0)