ONNX: TOCTOU arbitrary file read/write in save_external_dat

Summary

The save_external_data method seems to include multiple issues introducing a local TOCTOU vulnerability, an arbitrary file read/write on any system. It potentially includes a path validation bypass on Windows systems. Regarding the TOCTOU, an attacker seems to be able to overwrite victim's files via symlink following under the same privilege scope. The mentioned function can be found here: https://github.com/onnx/onnx/blob/main/onnx/external_data_helper.py#L188

Details

Toctou

The vulnerable code pattern:

   # CHECK - Is this a file?
   if not os.path.isfile(external_data_file_path):
       # Line 228-229: USE #1 - Create if it doesn't exist
       with open(external_data_file_path, "ab"):
           pass
   
   # Open for writing
   with open(external_data_file_path, "r+b") as data_file:
       # Lines 233-243: Write tensor data
       data_file.seek(0, 2)
       if info.offset is not None:
           file_size = data_file.tell()
           if info.offset > file_size:
               data_file.write(b"\0" * (info.offset - file_size))
           data_file.seek(info.offset)
       offset = data_file.tell()
       data_file.write(tensor.raw_data)

There is a time gap between os.path.isfile and open with no atomic file creation flags (e.g. O_EXCEL | O_CREAT) allowing the attacker to create a symlink that is being followed (absence of O_NOFOLLOW), between these two calls. By combining these, the attack is possible as shown below in the PoC section.

Bypass

There is also a potential validation bypass on Windows systems in the same method (https://github.com/onnx/onnx/blob/main/onnx/external_data_helper.py#L203) allowing absolute paths like (only 1 part):