net/tftp: Prevent a UAF and double-free from a failed seek
A malicious tftp server can cause UAFs and a double free. An attempt to read from a network file is handled by grub_net_fs_read(). If the read is at an offset other than the current offset, grub_net_seek_real() is invoked. In grub_net_seek_real(), if a backwards seek cannot be satisfied from the currently received packets, and the underlying transport does not provide a seek method, then grub_net_seek_real() will close and reopen the network protocol layer. For tftp, the ->close() call goes to tftp_close() and frees the tftp_data_t file->data. The file->data pointer is not nulled out after the free. If the ->open() call fails, the file->data will not be reallocated and will continue point to a freed memory block. This could happen from a server refusing to send the requisite ack to the new tftp request, for example. The seek and the read will then fail, but the grub_file continues to exist: the failed seek does not necessarily cause the entire file to be thrown away (e.g. where the file is checked to see if it is gzipped/lzio/xz/etc., a read failure is interpreted as a decompressor passing on the file, not as an invalidation of the entire grub_file_t structure). This means subsequent attempts to read or seek the file will use the old file->data after free. Eventually, the file will be close()d again and file->data will be freed again. Mark a net_fs file that doesn't reopen as broken. Do not permit read() or close() on a broken file (seek is not exposed directly to the file API - it is only called as part of read, so this blocks seeks as well). As an additional defence, null out the ->data pointer if tftp_open() fails. That would have lead to a simple null pointer dereference rather than a mess of UAFs. This may affect other protocols, I haven't checked. Signed-off-by:Daniel Axtens <dja@axtens.net> Reviewed-by:
Daniel Kiper <daniel.kiper@oracle.com>
Loading
Please register or sign in to comment