Just realized a better alternative method of using `!=` instead of `<`, will resubmit with that instead
2020/01/26 22:03、Evan Tang etang110@gmail.comのメール:
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
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; memcpy(head->magic,buf,LZ_MAGIC_LEN); memcpy(&(head->compressiontype),buf+LZ_MAGIC_LEN,1);
-- 2.15.2 (Apple Git-101.1)