[NordVPN Linux] [Bug]🐛Incorrect conversion between integer types

[odaysec]

nordvpn-linux/cmd/norduser/main.go

Line 291 in dbac8d8

fileshareManagementChan, fileshareShutdownChan := startFileshare(uint32(uid))

If a string is parsed into an int using strconv.Atoi, and subsequently that int is converted into another integer type of a smaller size, the result can produce unexpected values. This also applies to the results of strconv.ParseInt and strconv.ParseUint when the specified size is larger than the size of the type that number is converted to.

POC

In the first assume that an input string is passed to parseAllocateBad1 function, parsed by strconv.Atoi, and then converted into an int32 type:

package main

import (
	"strconv"
)

func parseAllocateBad1(wanted string) int32 {
	parsed, err := strconv.Atoi(wanted)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}
func parseAllocateBad2(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 64)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}

The bounds are not checked, so this means that if the provided number is greater than the maximum value of type int32, the resulting value from the conversion will be different from the actual provided value.

To avoid unexpected values, you should either use the other functions provided by the strconv package to parse the specific types and bit sizes as shown in the parseAllocateGood2 function; or check bounds as in the parseAllocateGood1 function.

package main

import (
	"math"
	"strconv"
)

func main() {

}

const DefaultAllocate int32 = 256

func parseAllocateGood1(desired string) int32 {
	parsed, err := strconv.Atoi(desired)
	if err != nil {
		return DefaultAllocate
	}
	// GOOD: check for lower and upper bounds
	if parsed > 0 && parsed <= math.MaxInt32 {
		return int32(parsed)
	}
	return DefaultAllocate
}
func parseAllocateGood2(desired string) int32 {
	// GOOD: parse specifying the bit size
	parsed, err := strconv.ParseInt(desired, 10, 32)
	if err != nil {
		return DefaultAllocate
	}
	return int32(parsed)
}

func parseAllocateGood3(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 32)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}
func parseAllocateGood4(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 64)
	if err != nil {
		panic(err)
	}
	// GOOD: check for lower and uppper bounds
	if parsed > 0 && parsed <= math.MaxInt32 {
		return int32(parsed)
	}
	return DefaultAllocate
}

In the second vulnerable, assume that an input string is passed to parseAllocateBad2 function, parsed by strconv.ParseInt with a bit size set to 64, and then converted into an int32 type:

package main

import (
	"strconv"
)

func parseAllocateBad1(wanted string) int32 {
	parsed, err := strconv.Atoi(wanted)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}
func parseAllocateBad2(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 64)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}

If the provided number is greater than the maximum value of type int32, the resulting value from the conversion will be different from the actual provided value.

To avoid unexpected values, you should specify the correct bit size as in parseAllocateGood3; or check bounds before making the conversion as in parseAllocateGood4.

package main

import (
	"math"
	"strconv"
)

func main() {

}

const DefaultAllocate int32 = 256

func parseAllocateGood1(desired string) int32 {
	parsed, err := strconv.Atoi(desired)
	if err != nil {
		return DefaultAllocate
	}
	// GOOD: check for lower and upper bounds
	if parsed > 0 && parsed <= math.MaxInt32 {
		return int32(parsed)
	}
	return DefaultAllocate
}
func parseAllocateGood2(desired string) int32 {
	// GOOD: parse specifying the bit size
	parsed, err := strconv.ParseInt(desired, 10, 32)
	if err != nil {
		return DefaultAllocate
	}
	return int32(parsed)
}

func parseAllocateGood3(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 32)
	if err != nil {
		panic(err)
	}
	return int32(parsed)
}
func parseAllocateGood4(wanted string) int32 {
	parsed, err := strconv.ParseInt(wanted, 10, 64)
	if err != nil {
		panic(err)
	}
	// GOOD: check for lower and uppper bounds
	if parsed > 0 && parsed <= math.MaxInt32 {
		return int32(parsed)
	}
	return DefaultAllocate
}

References

Wikipedia Integer overflow
Go language specification Integer overflow
Documentation for strconv.Atoi
Documentation for strconv.ParseInt
Documentation for strconv.ParseUint
Common Weakness Enumeration: CWE-190
Common Weakness Enumeration: CWE-681