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

validate/values.go:272 Division by zero #150

Closed
stasos24 opened this issue Dec 26, 2022 · 6 comments · Fixed by #151
Closed

validate/values.go:272 Division by zero #150

stasos24 opened this issue Dec 26, 2022 · 6 comments · Fixed by #151

Comments

@stasos24
Copy link

mult := data / factor

if
func MultipleOfInt(path, in string, data int64, factor int64) is called with factor = 0
You'll get division by zero at 272

@liggitt
Copy link
Contributor

liggitt commented Jan 3, 2023

should all numbers return true for "multiple of 0", or should all numbers return false for "multiple of 0", or should MultipleOf==0 be considered an error?

@stasos24
Copy link
Author

stasos24 commented Jan 3, 2023

should all numbers return true for "multiple of 0", or should all numbers return false for "multiple of 0", or should MultipleOf==0 be considered an error?

Return zero

@liggitt
Copy link
Contributor

liggitt commented Jan 3, 2023

I'm not sure what you mean by that... this would protect against division by zero by treating all numbers as divisible by zero:

diff --git a/values.go b/values.go
index c88d35d..72afe80 100644
--- a/values.go
+++ b/values.go
@@ -251,6 +251,9 @@ func MultipleOf(path, in string, data, factor float64) *errors.Validation {
 	if factor < 0 {
 		return errors.MultipleOfMustBePositive(path, in, factor)
 	}
+	if factor == 0 {
+		return nil
+	}
 	var mult float64
 	if factor < 1 {
 		mult = 1 / factor * data
@@ -269,6 +272,9 @@ func MultipleOfInt(path, in string, data int64, factor int64) *errors.Validation
 	if factor < 0 {
 		return errors.MultipleOfMustBePositive(path, in, factor)
 	}
+	if factor == 0 {
+		return nil
+	}
 	mult := data / factor
 	if mult*factor != data {
 		return errors.NotMultipleOf(path, in, factor, data)
@@ -278,6 +284,9 @@ func MultipleOfInt(path, in string, data int64, factor int64) *errors.Validation
 
 // MultipleOfUint validates if the provided unsigned integer is a multiple of the factor
 func MultipleOfUint(path, in string, data, factor uint64) *errors.Validation {
+	if factor == 0 {
+		return nil
+	}
 	mult := data / factor
 	if mult*factor != data {
 		return errors.NotMultipleOf(path, in, factor, data)

this would protect against it by requiring multipleOf to be positive:

diff --git a/values.go b/values.go
index c88d35d..e7ad8c1 100644
--- a/values.go
+++ b/values.go
@@ -248,7 +248,7 @@ func MinimumUint(path, in string, data, min uint64, exclusive bool) *errors.Vali
 // MultipleOf validates if the provided number is a multiple of the factor
 func MultipleOf(path, in string, data, factor float64) *errors.Validation {
 	// multipleOf factor must be positive
-	if factor < 0 {
+	if factor <= 0 {
 		return errors.MultipleOfMustBePositive(path, in, factor)
 	}
 	var mult float64
@@ -266,7 +266,7 @@ func MultipleOf(path, in string, data, factor float64) *errors.Validation {
 // MultipleOfInt validates if the provided integer is a multiple of the factor
 func MultipleOfInt(path, in string, data int64, factor int64) *errors.Validation {
 	// multipleOf factor must be positive
-	if factor < 0 {
+	if factor <= 0 {
 		return errors.MultipleOfMustBePositive(path, in, factor)
 	}
 	mult := data / factor
@@ -278,6 +278,10 @@ func MultipleOfInt(path, in string, data int64, factor int64) *errors.Validation
 
 // MultipleOfUint validates if the provided unsigned integer is a multiple of the factor
 func MultipleOfUint(path, in string, data, factor uint64) *errors.Validation {
+	// multipleOf factor must be positive
+	if factor == 0 {
+		return errors.MultipleOfMustBePositive(path, in, factor)
+	}
 	mult := data / factor
 	if mult*factor != data {
 		return errors.NotMultipleOf(path, in, factor, data)

@stasos24
Copy link
Author

stasos24 commented Jan 3, 2023

I am thinking first suggestion is more accurate

@stasos24
Copy link
Author

stasos24 commented Jan 3, 2023

It's look like a question of math. But in terms of code security both variants are valid

@liggitt
Copy link
Contributor

liggitt commented Jan 5, 2023

Opened #151 to fix this

I made all versions return errors, since MultipleOf already returned an error when evaluating division by 0.0

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 a pull request may close this issue.

2 participants