From 89385633ba1f6c6afbc304460d6385b05edb428d Mon Sep 17 00:00:00 2001
From: Petr Hosek <phosek@chromium.org>
Date: Mon, 22 Jul 2019 19:54:34 +0000
Subject: [PATCH] [libc++] Set __file_ to 0 in basic_filebuf::close() even if
 fclose fails

This issue was detected by ASan in one of our tests. This test manually
invokes basic_filebuf::cloe(). fclose(__h.release() returned a non-zero
exit status, so __file_ wasn't set to 0. Later when basic_filebuf
destructor ran, we would enter the if (__file_) block again leading to
heap-use-after-free error.

The POSIX specification for fclose says that independently of the return
value, fclose closes the underlying file descriptor and any further
access (including another call to fclose()) to the stream results in
undefined behavior. This is exactly what happened in our test case.

To avoid this issue, we have to always set __file_ to 0 independently of
the fclose return value.

Differential Revision: https://reviews.llvm.org/D64979

llvm-svn: 366730
---
 libcxx/include/fstream                             |  5 +-
 .../fstreams/filebuf.members/close.pass.cpp        | 56 ++++++++++++++++++++++
 2 files changed, 58 insertions(+), 3 deletions(-)
 create mode 100644 libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.pass.cpp

diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index 60a05b0..7db9017 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -697,10 +697,9 @@ basic_filebuf<_CharT, _Traits>::close()
         unique_ptr<FILE, int(*)(FILE*)> __h(__file_, fclose);
         if (sync())
             __rt = 0;
-        if (fclose(__h.release()) == 0)
-            __file_ = 0;
-        else
+        if (fclose(__h.release()))
             __rt = 0;
+        __file_ = 0;
         setbuf(0, 0);
     }
     return __rt;
diff --git a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.pass.cpp
new file mode 100644
index 0000000..b545041
--- /dev/null
+++ b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.pass.cpp
@@ -0,0 +1,56 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <fstream>
+
+// basic_filebuf<charT,traits>* close();
+
+#include <fstream>
+#include <cassert>
+#if defined(__unix__)
+#include <fcntl.h>
+#include <unistd.h>
+#endif
+#include "test_macros.h"
+#include "platform_support.h"
+
+int main(int, char**)
+{
+    std::string temp = get_temp_file_name();
+    {
+        std::filebuf f;
+        assert(!f.is_open());
+        assert(f.open(temp.c_str(), std::ios_base::out) != 0);
+        assert(f.is_open());
+        assert(f.close() != nullptr);
+        assert(!f.is_open());
+        assert(f.close() == nullptr);
+        assert(!f.is_open());
+    }
+#if defined(__unix__)
+    {
+        std::filebuf f;
+        assert(!f.is_open());
+        // Use open directly to get the file descriptor.
+        int fd = open(temp.c_str(), O_RDWR);
+        assert(fd >= 0);
+        // Use the internal method to create filebuf from the file descriptor.
+        assert(f.__open(fd, std::ios_base::out) != 0);
+        assert(f.is_open());
+        // Close the file descriptor directly to force filebuf::close to fail.
+        assert(close(fd) == 0);
+        // Ensure that filebuf::close handles the failure.
+        assert(f.close() == nullptr);
+        assert(!f.is_open());
+        assert(f.close() == nullptr);
+    }
+#endif
+    std::remove(temp.c_str());
+
+    return 0;
+}
-- 
2.7.4