Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Avoid float division in TOTP code #86

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KamilaBorowska
Copy link

@KamilaBorowska KamilaBorowska commented Sep 15, 2023

This mostly doesn't matter because 64-bit floats can represent 53-bit integers (and currently UNIX timestamps are 32 bits), but still, no reason to work with floats when integers will work fine.

This avoids Year 285618384 problem :).

@egonelbre
Copy link

Also, there's a slight bug on M1 processors due to float and uint conversions being platform dependent. https://go.dev/ref/spec#Conversions

On M1 Mac the uint64(float64) conversion can end up returning 0. Here's an example program.

package main

import (
	"math"
	"time"
)

func main() {
	period := uint(30)
	x := time.Time{}
	counter := int64(math.Floor(float64(x.Unix()) / float64(period)))
	counter2 := uint64(math.Floor(float64(x.Unix()) / float64(period)))
	println(counter, counter2)
	// Output: -2071186560 0
}

@KamilaBorowska
Copy link
Author

@egonelbre

TOTP algorithm doesn't support timestamps before Unix epoch (time.Time{} is year 1). Doesn't really matter as time moves forwards, so this shouldn't be possible in practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants