[PATCH] lzexpand: Fix uninitialized read in read_header

Evan Tang etang110 at gmail.com
Sun Jan 26 22:03:02 CST 2020

read_header calls _lread which can either return the number of characters read
or HFILE_ERROR (-1), casted to a UINT.  This was tested with a `< taget_length`
check, which doesn't catch HFILE_ERROR (since this is a UINT), allowing that case
to slip through

This could cause issues if LZInit is called in quick succession, first on a file
with a valid header (to put a valid LZ header in the buffer), then on an unreadable
file descriptor (a write-only one, for example), depending on if the compiler
decided to reuse the memory slot in the rest of LZInit

Signed-off-by: Evan Tang <etang110 at gmail.com>
An alternative method (which might be more obvious to someone reading the code) would be
to store the result of _lread in a variable and then explicitly check it both
for `< target_length` and `== HFILE_ERROR`, do you think we should do that instead?

Also, is this something we want regression tests for?  You could try
the sequence described above, which successfully fails on clang but not gcc
(due to how they compile the rest of LZInit).  I guess you could also
make the first file have an invalid compressiontype which would run much less
LZInit code and be more likely to fail, but in the end you'd still be relying
on undefined behavior doing something specific
 dlls/kernel32/lzexpand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/kernel32/lzexpand.c b/dlls/kernel32/lzexpand.c
index 4b2ec2ed45..471dba6c75 100644
--- a/dlls/kernel32/lzexpand.c
+++ b/dlls/kernel32/lzexpand.c
@@ -139,7 +139,7 @@ static INT read_header(HFILE fd,struct lzfileheader *head)
 	/* We can't directly read the lzfileheader struct due to
 	 * structure element alignment
-	if (_lread(fd,buf,LZ_HEADER_LEN)<LZ_HEADER_LEN)
+	if ((INT)_lread(fd,buf,LZ_HEADER_LEN)<LZ_HEADER_LEN)
 		return 0;
2.15.2 (Apple Git-101.1)

More information about the wine-devel mailing list