Skip to content

Commit 574c5cc

Browse files
committed
[libc++] Fix incorrect length check in std::basic_filebuf
This patch fixes an ASAN-found issue in std::basic_filebuf where we'd check the wrong size before proceeding to set our internal buffer to the externally-provided buffer, leading to the library trying to read from the incorrect buffer in underflow(). Thanks to Andrey Semin for the patch. Differential Revision: https://reviews.llvm.org/D154514
1 parent cb950c9 commit 574c5cc

File tree

4 files changed

+536
-1
lines changed

4 files changed

+536
-1
lines changed

libcxx/include/fstream

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -912,7 +912,7 @@ basic_filebuf<_CharT, _Traits>::setbuf(char_type* __s, streamsize __n)
912912
if (!__always_noconv_)
913913
{
914914
__ibs_ = max<streamsize>(__n, sizeof(__extbuf_min_));
915-
if (__s && __ibs_ >= sizeof(__extbuf_min_))
915+
if (__s && __ibs_ > sizeof(__extbuf_min_))
916916
{
917917
__intbuf_ = __s;
918918
__owns_ib_ = false;
Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
10+
// UNSUPPORTED: c++03
11+
12+
// <fstream>
13+
14+
// This test checks the behavior of reading payloads of different sizes in different patterns.
15+
// In particular, it was written to exercise code paths that deal with buffering inside the fstream
16+
// implementation.
17+
//
18+
// For each test, we test various behaviors w.r.t. how the buffer is handled:
19+
// - Provide a user-managed buffer to the library. In this case, we test the following corner-cases:
20+
// + A 0-sized buffer.
21+
// + A buffer size greater than and smaller than the payload size, which causes multiple buffer effects.
22+
// Important values are +/- 1 byte from the payload size.
23+
// - Let the library manage a buffer of a user-provided size 'n'. In this case, we test the following corner-cases:
24+
// + A 0-sized buffer.
25+
// + A buffer size greater than and smaller than the payload size, which causes multiple buffer effects.
26+
// Important values are +/- 1 or 2 bytes from the payload size.
27+
// + A buffer size smaller than 8 bytes. If pubsetbuf() is called with less than 8 bytes, the library will
28+
// use __extbuf_min_ with 8 bytes instead of allocating anything.
29+
// - Let the library manage a buffer, without specifying any size. In this case, the library will use the default
30+
// buffer size of 4096 bytes.
31+
32+
#include <cassert>
33+
#include <codecvt>
34+
#include <fstream>
35+
#include <locale>
36+
#include <numeric>
37+
#include <string>
38+
#include <vector>
39+
40+
#include "../types.h"
41+
#include "assert_macros.h"
42+
#include "platform_support.h"
43+
#include "test_macros.h"
44+
45+
template <class BufferPolicy>
46+
void test_read(BufferPolicy policy, const std::vector<std::streamsize>& payload_sizes) {
47+
std::streamsize total_size = std::accumulate(payload_sizes.begin(), payload_sizes.end(), 0);
48+
std::vector<char> data(total_size);
49+
for (std::size_t i = 0; i < data.size(); ++i) {
50+
data[i] = static_cast<char>(i % (1 << 8 * sizeof(char)));
51+
}
52+
std::string p = get_temp_file_name();
53+
{
54+
std::ofstream ofs;
55+
ofs.open(p, std::ios::out | std::ios::binary);
56+
assert(ofs.is_open());
57+
ofs.write(data.data(), data.size());
58+
assert(!ofs.fail());
59+
// test that the user's out_buffer buffer was not modified by write()
60+
for (std::streamsize j = 0; j < total_size; ++j) {
61+
char exp = j % (1 << 8 * sizeof(char));
62+
TEST_REQUIRE(data[j] == exp, [&] {
63+
test_eprintf("failed after write() at offset %zu: got=%x, expected=%x\n", j, data[j], exp);
64+
});
65+
}
66+
ofs.close();
67+
}
68+
{
69+
std::ifstream ifs;
70+
policy(ifs);
71+
ifs.open(p, std::ios::ate | std::ios::binary);
72+
assert(ifs.is_open());
73+
const std::streamsize in_sz = ifs.tellg();
74+
TEST_REQUIRE(in_sz == total_size, [&] { test_eprintf("out_sz = %zu, in_sz = %ld\n", total_size, in_sz); });
75+
ifs.seekg(0, std::ios::beg);
76+
std::size_t previously_read = 0;
77+
std::vector<char> in_buffer(total_size);
78+
for (const auto& payload_sz : payload_sizes) {
79+
ifs.read(in_buffer.data() + previously_read, payload_sz);
80+
assert(ifs);
81+
for (std::streamsize j = 0; j < payload_sz; ++j) {
82+
char exp = (previously_read + j) % (1 << 8 * sizeof(char));
83+
TEST_REQUIRE(in_buffer[previously_read + j] == exp, [&] {
84+
test_eprintf(
85+
"failed after read() at offset %zu (offset %zu in chunk size %zu): got=%x, expected=%x\n",
86+
previously_read + j,
87+
j,
88+
payload_sz,
89+
in_buffer[previously_read + j],
90+
exp);
91+
});
92+
}
93+
previously_read += payload_sz;
94+
}
95+
}
96+
std::remove(p.c_str());
97+
}
98+
99+
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
100+
template <class BufferPolicy>
101+
void test_read_codecvt(BufferPolicy policy, const std::vector<std::streamsize>& payload_sizes) {
102+
std::streamsize total_size = std::accumulate(payload_sizes.begin(), payload_sizes.end(), 0);
103+
std::vector<wchar_t> data(total_size);
104+
for (std::size_t i = 0; i < data.size(); ++i) {
105+
data[i] = static_cast<wchar_t>(i);
106+
}
107+
std::string p = get_temp_file_name();
108+
{
109+
std::wofstream ofs;
110+
ofs.imbue(std::locale(std::locale::classic(), new std::codecvt_utf8<wchar_t>));
111+
ofs.open(p, std::ios::out | std::ios::binary);
112+
assert(ofs.is_open());
113+
ofs.write(data.data(), data.size());
114+
assert(!ofs.fail());
115+
// test that the user's out_buffer buffer was not modified by write()
116+
for (std::streamsize j = 0; j < total_size; ++j) {
117+
wchar_t exp = static_cast<wchar_t>(j);
118+
TEST_REQUIRE(data[j] == exp, [&] {
119+
test_eprintf("failed after write() at offset %zu: got=%x, expected=%x\n", j, data[j], exp);
120+
});
121+
}
122+
ofs.close();
123+
}
124+
{
125+
std::wifstream ifs;
126+
ifs.imbue(std::locale(std::locale::classic(), new std::codecvt_utf8<wchar_t>));
127+
policy(ifs);
128+
ifs.open(p, std::ios::in | std::ios::binary);
129+
assert(ifs.is_open());
130+
ifs.seekg(0, std::ios::beg);
131+
std::size_t previously_read = 0;
132+
std::vector<wchar_t> in_buffer(total_size);
133+
for (const auto& payload_sz : payload_sizes) {
134+
assert(ifs.read(in_buffer.data() + previously_read, payload_sz));
135+
assert(ifs);
136+
for (std::streamsize j = 0; j < payload_sz; ++j) {
137+
wchar_t exp = static_cast<wchar_t>(j + previously_read);
138+
TEST_REQUIRE(in_buffer[j + previously_read] == exp, [&] {
139+
test_eprintf(
140+
"failed after read() at offset %zu: got=%x, expected=%x\n", j, in_buffer[j + previously_read], exp);
141+
});
142+
}
143+
previously_read += payload_sz;
144+
}
145+
}
146+
std::remove(p.c_str());
147+
}
148+
#endif
149+
150+
const std::vector<std::streamsize> buffer_sizes{0L, 3L, 8L, 9L, 11L};
151+
const std::vector<std::streamsize> io_sizes{0L, 1L, 2L, 3L, 4L, 9L, 10L, 11L, 12L, 13L, 21L, 22L, 23L};
152+
const std::vector<std::streamsize> io_sizes_default{
153+
0L, 1L, 2L, 3L, 4L, 4094L, 4095L, 4096L, 4097L, 4098L, 8190L, 8191L, 8192L, 8193L, 8194L};
154+
155+
// Test single read operations
156+
void test_1_read() {
157+
// with default library buffer size: 4096b
158+
for (std::streamsize x : io_sizes_default) {
159+
test_read(LibraryDefaultBuffer(), {x});
160+
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
161+
test_read_codecvt(LibraryDefaultBuffer(), {x});
162+
#endif
163+
}
164+
165+
// with the library-managed buffer of given size
166+
for (std::streamsize b : buffer_sizes) {
167+
for (std::streamsize x : io_sizes) {
168+
test_read(LibraryManagedBuffer(b), {x});
169+
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
170+
test_read_codecvt(LibraryManagedBuffer(b), {x});
171+
#endif
172+
}
173+
}
174+
175+
// with the user-managed buffer of given size
176+
for (std::streamsize b : buffer_sizes) {
177+
for (std::streamsize x : io_sizes) {
178+
test_read(UserManagedBuffer(b), {x});
179+
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
180+
test_read_codecvt(UserManagedBuffer(b), {x});
181+
#endif
182+
}
183+
}
184+
}
185+
186+
// Test two read operations
187+
void test_2_reads() {
188+
// with default library buffer size: 4096b
189+
for (std::streamsize a : io_sizes_default) {
190+
for (std::streamsize b : io_sizes_default) {
191+
test_read(LibraryDefaultBuffer(), {a, b});
192+
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
193+
test_read_codecvt(LibraryDefaultBuffer(), {a, b});
194+
#endif
195+
}
196+
}
197+
198+
// with the library-managed buffer of given size
199+
for (std::streamsize buf : buffer_sizes) {
200+
for (std::streamsize a : io_sizes) {
201+
for (std::streamsize b : io_sizes) {
202+
test_read(LibraryManagedBuffer(buf), {a, b});
203+
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
204+
test_read_codecvt(LibraryManagedBuffer(buf), {a, b});
205+
#endif
206+
}
207+
}
208+
}
209+
210+
// with the user-managed buffer of given size
211+
for (std::streamsize buf : buffer_sizes) {
212+
for (std::streamsize a : io_sizes) {
213+
for (std::streamsize b : io_sizes) {
214+
test_read(UserManagedBuffer(buf), {a, b});
215+
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
216+
test_read_codecvt(UserManagedBuffer(buf), {a, b});
217+
#endif
218+
}
219+
}
220+
}
221+
}
222+
223+
int main(int, char**) {
224+
test_1_read();
225+
test_2_reads();
226+
return 0;
227+
}

0 commit comments

Comments
 (0)