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

Panic within QUICRandomFrames #49

Open
Diniboy1123 opened this issue Dec 20, 2024 · 1 comment · Fixed by #55
Open

Panic within QUICRandomFrames #49

Diniboy1123 opened this issue Dec 20, 2024 · 1 comment · Fixed by #55
Labels
bug Something isn't working

Comments

@Diniboy1123
Copy link

Hey,

This is a great library, however there seems to be an issue with the QUICRandomFrames algorithm I am sometimes running into. It throws: panic: crypto/rand: argument to Int is <= 0

Based on the stacktrace it occurs at this line:

lenCRYPTO, err := cryptoSafeRandUint64(1, lenCryptoData-(numCRYPTO-i-2))

I modified the surrounding code blindly to avoid overflows:

lenCryptoData := uint64(len(cryptoData))
offsetCryptoData := uint64(0)
for i := uint64(0); i < numCRYPTO-1; i++ { // select n-1 times, since the last one must be the remaining
	// randomly select length of CRYPTO frame.
	// Length must be at least 1 byte and at most the remaining length of cryptoData minus the remaining number of CRYPTO frames.
	// i.e. len in [1, len(cryptoData)-offsetCryptoData-(numCRYPTO-i-2))
	log.Printf("lenCryptoData: %d, offsetCryptoData: %d, numCRYPTO: %d, i: %d\n", lenCryptoData, offsetCryptoData, numCRYPTO, i)
	remainingCrypto := lenCryptoData - (numCRYPTO - i - 2)

	// check if it overflows uint64
	if remainingCrypto > 0xFFFFFFFF || remainingCrypto == 0 {
		log.Printf("remainingCrypto overflow: %d\n", remainingCrypto)
		break
	} else {
		log.Printf("remainingCrypto: %d\n", remainingCrypto)
	}

	lenCRYPTO, err := cryptoSafeRandUint64(1, remainingCrypto)
	if err != nil {
		return nil, err
	}
	frameList = append(frameList, QUICFrameCrypto{Offset: int(offsetCryptoData), Length: int(lenCRYPTO)})
	offsetCryptoData += lenCRYPTO
	lenCryptoData -= lenCRYPTO
}

And in the problematic case I saw this in the logs:

2024/12/20 03:59:19 lenCryptoData: 508, offsetCryptoData: 0, numCRYPTO: 5, i: 0
2024/12/20 03:59:19 remainingCrypto: 505
2024/12/20 03:59:19 lenCryptoData: 69, offsetCryptoData: 439, numCRYPTO: 5, i: 1
2024/12/20 03:59:19 remainingCrypto: 67
2024/12/20 03:59:19 lenCryptoData: 54, offsetCryptoData: 454, numCRYPTO: 5, i: 2
2024/12/20 03:59:19 remainingCrypto: 53
2024/12/20 03:59:19 lenCryptoData: 31, offsetCryptoData: 477, numCRYPTO: 5, i: 3
2024/12/20 03:59:19 remainingCrypto: 31
2024/12/20 03:59:19 lenCryptoData: 0, offsetCryptoData: 0, numCRYPTO: 6, i: 0
2024/12/20 03:59:19 remainingCrypto overflow: 18446744073709551612

It doesn't always happen, but when it does, the program panics...

@mingyech mingyech added the bug Something isn't working label Jan 18, 2025
@mingyech
Copy link
Member

Wait I just realized that this is a different issue from the PR above 🤦

@mingyech mingyech reopened this Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants