[PATCH v2] kernel32/lzexpand: Fix uninitialized read in read_header

[Evan Tang]

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>
---
v2: Realized the best solution is to switch the < to a != since we're not
expecting the result to be > target_length either

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..73eaf34a3e 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 (_lread(fd,buf,LZ_HEADER_LEN)!=LZ_HEADER_LEN)
 		return 0;
 	memcpy(head->magic,buf,LZ_MAGIC_LEN);
 	memcpy(&(head->compressiontype),buf+LZ_MAGIC_LEN,1);
-- 
2.15.2 (Apple Git-101.1)