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@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);