Page MenuHome GnuPG

result of fread() is assigned to an int variable
Closed, ResolvedPublic

Description

libksba's function ksba_reader_read() stores the result of an fread() call into an int variable n:

https://git.gnupg.org/cgi-bin/gitweb.cgi?
p=libksba.git;a=blob;f=src/reader.c;h=0f8bad50bb189e1f1cd8438038dcfaa1f0fe0c58;hb=995d2e34932143cc9888db779cb3ecd92ae6
e32e#l380

The fread() function returns a size_t. On 64-bit architectures where int is 32-bit wide and size_t is 64-bit wide,
this means that requests for 0x80000001, 0x100000000 or respectively for 0x100000001 bytes cause an overflow in the
assignment of the result of the fread() call to n. In all three cases the value of n does not reflect what the fread()
call does.

In the first case, the condition n > 0 at line 381 is made true by the overflow and, interestingly, the condition n <
length at line 388 is made false (in this comparison of a signed int and a wider unsigned size_t, the signed operand
is converted to unsigned, and becomes very large, larger than length). This implies that r->eof is not set:
https://git.gnupg.org/cgi-bin/gitweb.cgi?
p=libksba.git;a=blob;f=src/reader.c;h=0f8bad50bb189e1f1cd8438038dcfaa1f0fe0c58;hb=995d2e34932143cc9888db779cb3ecd92ae6
e32e#l388

In the second case, the condition n > 0 is made false and the condition n < length is made true.

In the third case, the condition n > 0 is made true and n < length is made true.

Despite the somewhat wide range of mis-behaviors, there does not appear to be any security consequences to this bug
when reading a file that an attacker can make 2GiB or 4GiB-large at their leisure.

Still, there is no reason not to make the n variable a size_t to match the type returned by fread(). For clarity, the
condition n <= 0 at line 393 should be made n == 0 on the same occasion:

https://git.gnupg.org/cgi-bin/gitweb.cgi?
p=libksba.git;a=blob;f=src/reader.c;h=0f8bad50bb189e1f1cd8438038dcfaa1f0fe0c58;hb=995d2e34932143cc9888db779cb3ecd92ae6
e32e#l393

Details

Version
1.3.4