VirusTotal / yara

The pattern matching swiss knife
https://virustotal.github.io/yara/
BSD 3-Clause "New" or "Revised" License
8.08k stars 1.43k forks source link

Implicit widening of multiplication result can cause unintended overflow #1970

Closed philipjonsen closed 8 months ago

philipjonsen commented 11 months ago

DESCRIPTION:

If a result of a multiplication expression is implicitly widened the result might overflow. If the widening is intentional consider using explicit typecast. Otherwise, perform the multiplication in a wider type to avoid the implicit widening of the result. This is mainly useful when operating on very large buffers.

For example, consider the following code:

void zeroinit(char base, unsigned width, unsigned height) { for(unsigned row = 0; row != height; ++row) { for(unsigned col = 0; col != width; ++col) { char ptr = base + row width + col; ptr = 0; } } } This is fine in general, but if width * height overflows, you end up wrapping back to the beginning of base instead of processing the entire requested buffer. Indeed, this only matters for pretty large buffers, but that is not unheard of in the areas such as image processing.

BAD PRACTICE:

long mul(int a, int b) { return a * b; // warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int' }

char ptr_add(char base, int a, int b) { return base + a * b; // warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t' }

char ptr_subscript(char base, int a, int b) { return base[a b]; // warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t' } In the mul function, the multiplication of a and b is performed in type int, but the result is implicitly widened to type long. To fix this, you can either explicitly widen the multiplication by casting one of the operands to long, or perform the multiplication in a wider type, such as long.

In the ptr_add and ptr_subscript functions, the result of the multiplication of a and b is used as a pointer offset after an implicit widening conversion to type ssize_t. This can lead to unexpected behavior if the multiplication overflows. To fix this, you can either explicitly widen the multiplication or perform the multiplication in a wider type, such as ssize_t.

RECOMMENDED:

long mul(int a, int b) { return static_cast(a) * b; }

char ptr_add(char base, int a, int b) { return base + static_cast(a * b); }

char ptr_subscript(char base, int a, int b) { return base[static_cast(a b)]; } In the mul function, the multiplication is explicitly widened by casting a to long. This ensures that the multiplication is performed in type long and avoids the implicit widening conversion.

In the ptr_add and ptr_subscript functions, the multiplication is explicitly cast to ssize_t before using it as a pointer offset. This ensures that the multiplication is performed in type ssize_t and avoids the implicit widening conversion.

Result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ptrdiff_t' can be found here: yara/blob/master/libyara/atoms.c#L832-L832

performing an implicit widening conversion to type 'int64_t' (aka 'long') of a multiplication performed in type 'int' can be found here: yara/blob/master/libyara/exec.c#L851-L851