mirror of
https://github.com/github/codeql.git
synced 2026-04-30 11:15:13 +02:00
CWE-681: initial commit
This commit is contained in:
Slavomir
13
ql/src/Security/CWE-681/IncorrectNumericConversion.go
Normal file
13
ql/src/Security/CWE-681/IncorrectNumericConversion.go
Normal file
@@ -0,0 +1,13 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"strconv"
|
||||
)
|
||||
|
||||
func parseAllocate(wanted string) int32 {
|
||||
parsed, err := strconv.Atoi(wanted)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
return int32(parsed)
|
||||
}
|
||||
48
ql/src/Security/CWE-681/IncorrectNumericConversion.qhelp
Normal file
48
ql/src/Security/CWE-681/IncorrectNumericConversion.qhelp
Normal file
@@ -0,0 +1,48 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
If a numeric value string is parsed using <code>strconv.Atoi</code> into an int, and then that int
|
||||
is converted into another type of a lower bit size, the result can produce unexpected values.
|
||||
</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
If you need to parse numeric values with specific bit sizes, use the functions specific to each
|
||||
type (<code>strconv.ParseFloat</code>, <code>strconv.ParseInt</code>, <code>strconv.ParseUint</code>)
|
||||
that also allow to specify the wanted bit size.
|
||||
</p>
|
||||
<p>
|
||||
If this is not possible, then add upper (and lower) bound checks specific to each type and
|
||||
bit size (you can find the min and max value for each type in the `math` package).
|
||||
</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>
|
||||
In the following example, assume that an input string is passed to <code>parseAllocate</code> function,
|
||||
parsed by <code>strconv.Atoi</code>, and then converted into an <code>int32</code> type:
|
||||
</p>
|
||||
<sample src="IncorrectNumericConversion.go"/>
|
||||
<p>
|
||||
The bounds are not checked, so this means that if the provided number is greater than max int32,
|
||||
the resulting value from the conversion will be different from the provided value.
|
||||
</p>
|
||||
<p>
|
||||
To avoid unexpected values, you should either use the other functions provided by the <code>strconv</code>
|
||||
package to parse the specific types and bit sizes; in this case, <code>strconv.ParseInt</code> as you
|
||||
can see in <code>parseAllocateGood2</code> function; or check bounds as in <code>parseAllocateGood1</code>
|
||||
function.
|
||||
</p>
|
||||
<sample src="IncorrectNumericConversionGood.go"/>
|
||||
</example>
|
||||
<references>
|
||||
<li>
|
||||
mitre.org: <a href="https://cwe.mitre.org/data/definitions/681.html">CWE-681: Incorrect Conversion between Numeric Types</a>.
|
||||
</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
141
ql/src/Security/CWE-681/IncorrectNumericConversion.ql
Normal file
141
ql/src/Security/CWE-681/IncorrectNumericConversion.ql
Normal file
@@ -0,0 +1,141 @@
|
||||
/**
|
||||
* @name Incorrect Conversion between Numeric Types
|
||||
* @description Converting the result of strconv.Atoi (which is of type `int`) to
|
||||
* numeric types of lower bit size can produce unexpected values.
|
||||
* @kind path-problem
|
||||
* @id go/incorrect-numeric-conversion
|
||||
* @tags security
|
||||
* external/cwe/cwe-681
|
||||
*/
|
||||
|
||||
import go
|
||||
import DataFlow::PathGraph
|
||||
|
||||
class IntParser extends Function {
|
||||
IntParser() { this.hasQualifiedName("strconv", "Atoi") }
|
||||
}
|
||||
|
||||
class OverflowingConversionExpr extends ConversionExpr {
|
||||
string conversionTypeName;
|
||||
|
||||
OverflowingConversionExpr() {
|
||||
exists(ConversionExpr conv |
|
||||
conversionTypeName = conv.getTypeExpr().getType().getUnderlyingType*().getName() and
|
||||
(
|
||||
// anything lower than int64:
|
||||
conversionTypeName = "int8" or
|
||||
conversionTypeName = "int16" or
|
||||
conversionTypeName = "int32" or
|
||||
// anything lower than uint64:
|
||||
conversionTypeName = "uint8" or
|
||||
conversionTypeName = "uint16" or
|
||||
conversionTypeName = "uint32" or
|
||||
// anything lower than float64:
|
||||
conversionTypeName = "float32"
|
||||
)
|
||||
|
|
||||
this = conv
|
||||
)
|
||||
}
|
||||
|
||||
string getTypeName() { result = conversionTypeName }
|
||||
}
|
||||
|
||||
class IfRelationalComparison extends IfStmt {
|
||||
IfRelationalComparison() {
|
||||
this.getCond() instanceof RelationalComparisonExpr or this.getCond() instanceof LandExpr
|
||||
}
|
||||
|
||||
RelationalComparisonExpr getComparison() { result = this.getCond().(RelationalComparisonExpr) }
|
||||
|
||||
LandExpr getLandExpr() { result = this.getCond().(LandExpr) }
|
||||
}
|
||||
|
||||
class FlowConfig extends TaintTracking::Configuration, DataFlow::Configuration {
|
||||
FlowConfig() { this = "FlowConfig" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) {
|
||||
exists(IntParser atoi | source = atoi.getACall().getResult(0))
|
||||
}
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(OverflowingConversionExpr conv | sink.asExpr() = conv)
|
||||
}
|
||||
|
||||
override predicate isSanitizerIn(DataFlow::Node node) {
|
||||
// If the conversion is inside an `if` block that compares the
|
||||
// source as `source > 0`, then that sanitizes conversion of int64 to int32;
|
||||
exists(IfRelationalComparison san, OverflowingConversionExpr conv |
|
||||
conv = node.asExpr().(OverflowingConversionExpr) and
|
||||
san.getThen().getAChild*() = conv and
|
||||
(
|
||||
conv.getTypeName() = "int32" and
|
||||
san.getComparison().getLesserOperand().getNumericValue() = 0 and
|
||||
san.getComparison().getGreaterOperand().getGlobalValueNumber() =
|
||||
conv.getOperand().getGlobalValueNumber()
|
||||
or
|
||||
comparisonGreaterOperandIsEqualOrLess("int8", san, conv, getMaxInt8())
|
||||
or
|
||||
comparisonGreaterOperandIsEqualOrLess("int16", san, conv, getMaxInt16())
|
||||
or
|
||||
comparisonGreaterOperandIsEqualOrLess("int32", san, conv, getMaxInt32())
|
||||
or
|
||||
comparisonGreaterOperandIsEqualOrLess("uint8", san, conv, getMaxUint8())
|
||||
or
|
||||
comparisonGreaterOperandIsEqualOrLess("uint16", san, conv, getMaxUint16())
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
int getMaxInt8() {
|
||||
result = 2.pow(7) - 1
|
||||
// = 1<<7 - 1
|
||||
}
|
||||
|
||||
int getMaxInt16() {
|
||||
result = 2.pow(15) - 1
|
||||
// = 1<<15 - 1
|
||||
}
|
||||
|
||||
int getMaxInt32() {
|
||||
result = 2.pow(31) - 1
|
||||
// = 1<<31 - 1
|
||||
}
|
||||
|
||||
int getMaxUint8() {
|
||||
result = 2.pow(8) - 1
|
||||
// = 1<<8 - 1
|
||||
}
|
||||
|
||||
int getMaxUint16() {
|
||||
result = 2.pow(16) - 1
|
||||
// = 1<<16 - 1
|
||||
}
|
||||
|
||||
predicate comparisonGreaterOperandIsEqualOrLess(
|
||||
string typeName, IfRelationalComparison ifExpr, OverflowingConversionExpr conv, int value
|
||||
) {
|
||||
conv.getTypeName() = typeName and
|
||||
(
|
||||
// exclude cases like: if parsed < math.MaxInt8 { int8(parsed)}
|
||||
ifExpr.getComparison().getGreaterOperand().getNumericValue() = value and
|
||||
// and lesser is the conversion operand:
|
||||
ifExpr.getComparison().getLesserOperand().getGlobalValueNumber() =
|
||||
conv.getOperand().getGlobalValueNumber()
|
||||
or
|
||||
// exclude cases like: if err == nil && parsed < math.MaxInt8 { int8(parsed)}
|
||||
exists(RelationalComparisonExpr andExpr |
|
||||
andExpr = ifExpr.getLandExpr().getAnOperand().(RelationalComparisonExpr)
|
||||
|
|
||||
andExpr.getGreaterOperand().getNumericValue() = value and
|
||||
// and lesser is the conversion operand:
|
||||
andExpr.getLesserOperand().getGlobalValueNumber() = conv.getOperand().getGlobalValueNumber()
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
from FlowConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
|
||||
where cfg.hasFlowPath(source, sink)
|
||||
select source, source, sink,
|
||||
"Incorrect type conversion of int from strconv.Atoi result to another numeric type"
|
||||
32
ql/src/Security/CWE-681/IncorrectNumericConversionGood.go
Normal file
32
ql/src/Security/CWE-681/IncorrectNumericConversionGood.go
Normal file
@@ -0,0 +1,32 @@
|
||||
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 uppper 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)
|
||||
}
|
||||
@@ -0,0 +1,43 @@
|
||||
edges
|
||||
| IncorrectNumericConversion.go:26:2:26:28 | ... := ...[0] : int | IncorrectNumericConversion.go:35:41:35:50 | type conversion |
|
||||
| IncorrectNumericConversion.go:54:3:54:36 | ... := ...[0] : int | IncorrectNumericConversion.go:58:7:58:18 | type conversion |
|
||||
| IncorrectNumericConversion.go:61:3:61:36 | ... := ...[0] : int | IncorrectNumericConversion.go:65:7:65:19 | type conversion |
|
||||
| IncorrectNumericConversion.go:68:3:68:36 | ... := ...[0] : int | IncorrectNumericConversion.go:72:7:72:19 | type conversion |
|
||||
| IncorrectNumericConversion.go:75:3:75:36 | ... := ...[0] : int | IncorrectNumericConversion.go:79:7:79:19 | type conversion |
|
||||
| IncorrectNumericConversion.go:82:3:82:36 | ... := ...[0] : int | IncorrectNumericConversion.go:86:7:86:20 | type conversion |
|
||||
| IncorrectNumericConversion.go:89:3:89:36 | ... := ...[0] : int | IncorrectNumericConversion.go:93:7:93:20 | type conversion |
|
||||
| IncorrectNumericConversion.go:96:3:96:36 | ... := ...[0] : int | IncorrectNumericConversion.go:100:7:100:21 | type conversion |
|
||||
| IncorrectNumericConversion.go:103:3:103:36 | ... := ...[0] : int | IncorrectNumericConversion.go:108:7:108:18 | type conversion |
|
||||
| IncorrectNumericConversion.go:112:3:112:36 | ... := ...[0] : int | IncorrectNumericConversion.go:116:7:116:23 | type conversion |
|
||||
nodes
|
||||
| IncorrectNumericConversion.go:26:2:26:28 | ... := ...[0] : int | semmle.label | ... := ...[0] : int |
|
||||
| IncorrectNumericConversion.go:35:41:35:50 | type conversion | semmle.label | type conversion |
|
||||
| IncorrectNumericConversion.go:54:3:54:36 | ... := ...[0] : int | semmle.label | ... := ...[0] : int |
|
||||
| IncorrectNumericConversion.go:58:7:58:18 | type conversion | semmle.label | type conversion |
|
||||
| IncorrectNumericConversion.go:61:3:61:36 | ... := ...[0] : int | semmle.label | ... := ...[0] : int |
|
||||
| IncorrectNumericConversion.go:65:7:65:19 | type conversion | semmle.label | type conversion |
|
||||
| IncorrectNumericConversion.go:68:3:68:36 | ... := ...[0] : int | semmle.label | ... := ...[0] : int |
|
||||
| IncorrectNumericConversion.go:72:7:72:19 | type conversion | semmle.label | type conversion |
|
||||
| IncorrectNumericConversion.go:75:3:75:36 | ... := ...[0] : int | semmle.label | ... := ...[0] : int |
|
||||
| IncorrectNumericConversion.go:79:7:79:19 | type conversion | semmle.label | type conversion |
|
||||
| IncorrectNumericConversion.go:82:3:82:36 | ... := ...[0] : int | semmle.label | ... := ...[0] : int |
|
||||
| IncorrectNumericConversion.go:86:7:86:20 | type conversion | semmle.label | type conversion |
|
||||
| IncorrectNumericConversion.go:89:3:89:36 | ... := ...[0] : int | semmle.label | ... := ...[0] : int |
|
||||
| IncorrectNumericConversion.go:93:7:93:20 | type conversion | semmle.label | type conversion |
|
||||
| IncorrectNumericConversion.go:96:3:96:36 | ... := ...[0] : int | semmle.label | ... := ...[0] : int |
|
||||
| IncorrectNumericConversion.go:100:7:100:21 | type conversion | semmle.label | type conversion |
|
||||
| IncorrectNumericConversion.go:103:3:103:36 | ... := ...[0] : int | semmle.label | ... := ...[0] : int |
|
||||
| IncorrectNumericConversion.go:108:7:108:18 | type conversion | semmle.label | type conversion |
|
||||
| IncorrectNumericConversion.go:112:3:112:36 | ... := ...[0] : int | semmle.label | ... := ...[0] : int |
|
||||
| IncorrectNumericConversion.go:116:7:116:23 | type conversion | semmle.label | type conversion |
|
||||
#select
|
||||
| IncorrectNumericConversion.go:26:2:26:28 | ... := ...[0] : int | IncorrectNumericConversion.go:26:2:26:28 | ... := ...[0] : int | IncorrectNumericConversion.go:35:41:35:50 | type conversion | Incorrect type conversion of int from strconv.Atoi result to another numeric type |
|
||||
| IncorrectNumericConversion.go:54:3:54:36 | ... := ...[0] : int | IncorrectNumericConversion.go:54:3:54:36 | ... := ...[0] : int | IncorrectNumericConversion.go:58:7:58:18 | type conversion | Incorrect type conversion of int from strconv.Atoi result to another numeric type |
|
||||
| IncorrectNumericConversion.go:61:3:61:36 | ... := ...[0] : int | IncorrectNumericConversion.go:61:3:61:36 | ... := ...[0] : int | IncorrectNumericConversion.go:65:7:65:19 | type conversion | Incorrect type conversion of int from strconv.Atoi result to another numeric type |
|
||||
| IncorrectNumericConversion.go:68:3:68:36 | ... := ...[0] : int | IncorrectNumericConversion.go:68:3:68:36 | ... := ...[0] : int | IncorrectNumericConversion.go:72:7:72:19 | type conversion | Incorrect type conversion of int from strconv.Atoi result to another numeric type |
|
||||
| IncorrectNumericConversion.go:75:3:75:36 | ... := ...[0] : int | IncorrectNumericConversion.go:75:3:75:36 | ... := ...[0] : int | IncorrectNumericConversion.go:79:7:79:19 | type conversion | Incorrect type conversion of int from strconv.Atoi result to another numeric type |
|
||||
| IncorrectNumericConversion.go:82:3:82:36 | ... := ...[0] : int | IncorrectNumericConversion.go:82:3:82:36 | ... := ...[0] : int | IncorrectNumericConversion.go:86:7:86:20 | type conversion | Incorrect type conversion of int from strconv.Atoi result to another numeric type |
|
||||
| IncorrectNumericConversion.go:89:3:89:36 | ... := ...[0] : int | IncorrectNumericConversion.go:89:3:89:36 | ... := ...[0] : int | IncorrectNumericConversion.go:93:7:93:20 | type conversion | Incorrect type conversion of int from strconv.Atoi result to another numeric type |
|
||||
| IncorrectNumericConversion.go:96:3:96:36 | ... := ...[0] : int | IncorrectNumericConversion.go:96:3:96:36 | ... := ...[0] : int | IncorrectNumericConversion.go:100:7:100:21 | type conversion | Incorrect type conversion of int from strconv.Atoi result to another numeric type |
|
||||
| IncorrectNumericConversion.go:103:3:103:36 | ... := ...[0] : int | IncorrectNumericConversion.go:103:3:103:36 | ... := ...[0] : int | IncorrectNumericConversion.go:108:7:108:18 | type conversion | Incorrect type conversion of int from strconv.Atoi result to another numeric type |
|
||||
| IncorrectNumericConversion.go:112:3:112:36 | ... := ...[0] : int | IncorrectNumericConversion.go:112:3:112:36 | ... := ...[0] : int | IncorrectNumericConversion.go:116:7:116:23 | type conversion | Incorrect type conversion of int from strconv.Atoi result to another numeric type |
|
||||
@@ -0,0 +1,201 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"math"
|
||||
"strconv"
|
||||
)
|
||||
|
||||
func main() {
|
||||
|
||||
}
|
||||
|
||||
type Something struct {
|
||||
}
|
||||
type Config struct {
|
||||
}
|
||||
type Registry struct {
|
||||
}
|
||||
|
||||
func LookupTarget(conf *Config, num int32) (int32, error) {
|
||||
return 567, nil
|
||||
}
|
||||
func LookupNumberByName(reg *Registry, name string) (int32, error) {
|
||||
return 567, nil
|
||||
}
|
||||
func lab(s string) (*Something, error) {
|
||||
num, err := strconv.Atoi(s)
|
||||
|
||||
if err != nil {
|
||||
number, err := LookupNumberByName(&Registry{}, s)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
num = int(number)
|
||||
}
|
||||
target, err := LookupTarget(&Config{}, int32(num))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// convert the resolved target number back to a string
|
||||
|
||||
s = strconv.Itoa(int(target))
|
||||
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
const CustomMaxInt16 = 1<<15 - 1
|
||||
|
||||
type CustomInt int16
|
||||
|
||||
// these should be caught:
|
||||
func upperBoundIsNOTChecked(input string) {
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
_ = int8(parsed)
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
_ = int16(parsed)
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
_ = int32(parsed)
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
_ = uint8(parsed)
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
_ = uint16(parsed)
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
_ = uint32(parsed)
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
_ = float32(parsed)
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
// NOTE: byte is uint8
|
||||
_ = byte(parsed)
|
||||
}
|
||||
{
|
||||
// using custom type:
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
_ = CustomInt(parsed)
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
// these should NOT be caught:
|
||||
func upperBoundIsChecked(input string) {
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
if parsed < math.MaxInt8 {
|
||||
_ = int8(parsed)
|
||||
}
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
if parsed < math.MaxInt16 {
|
||||
_ = int16(parsed)
|
||||
}
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
if parsed > 0 {
|
||||
_ = int32(parsed)
|
||||
}
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
if parsed < math.MaxInt32 {
|
||||
_ = int32(parsed)
|
||||
}
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
if parsed < math.MaxUint8 {
|
||||
_ = uint8(parsed)
|
||||
}
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
if parsed < math.MaxUint16 {
|
||||
_ = uint16(parsed)
|
||||
}
|
||||
}
|
||||
{
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
if parsed < math.MaxUint8 {
|
||||
_ = byte(parsed)
|
||||
}
|
||||
}
|
||||
{ // multiple `and` conditions
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err == nil && 1 == 1 && parsed < math.MaxInt8 {
|
||||
_ = int8(parsed)
|
||||
}
|
||||
}
|
||||
{ // custom maxInt16
|
||||
parsed, err := strconv.Atoi(input)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
if parsed < CustomMaxInt16 {
|
||||
_ = int16(parsed)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
Security/CWE-681/IncorrectNumericConversion.ql
|
||||
Reference in New Issue
Block a user