Skip to content

Commit c88d8a2

Browse files
implicitfieldADKaster
authored andcommitted
LibArchive: Make TarInputStream::advance report errors
Fixes this bug that was reported by OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=52862
1 parent 26a4327 commit c88d8a2

File tree

4 files changed

+18
-8
lines changed

4 files changed

+18
-8
lines changed

Meta/Lagom/Fuzzers/FuzzTar.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
1616
if (!tar_stream.valid())
1717
return 0;
1818

19-
for (; !tar_stream.finished(); tar_stream.advance()) {
19+
while (!tar_stream.finished()) {
2020
auto const& header = tar_stream.header();
2121

2222
if (!header.content_is_like_extended_header())
@@ -33,6 +33,10 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
3333
default:
3434
return 0;
3535
}
36+
37+
auto maybe_error = tar_stream.advance();
38+
if (maybe_error.is_error())
39+
return 0;
3640
}
3741

3842
return 0;

Userland/Libraries/LibArchive/TarStream.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ TarInputStream::TarInputStream(InputStream& stream)
7171
{
7272
if (!m_stream.read_or_error(Bytes(&m_header, sizeof(m_header)))) {
7373
m_finished = true;
74-
m_stream.handle_any_error(); // clear out errors so we dont assert
74+
m_stream.handle_any_error(); // clear out errors so we don't assert
7575
return;
7676
}
7777
VERIFY(m_stream.discard_or_error(block_size - sizeof(TarFileHeader)));
@@ -82,25 +82,27 @@ static constexpr unsigned long block_ceiling(unsigned long offset)
8282
return block_size * (1 + ((offset - 1) / block_size));
8383
}
8484

85-
void TarInputStream::advance()
85+
ErrorOr<void> TarInputStream::advance()
8686
{
8787
if (m_finished)
88-
return;
88+
return Error::from_string_literal("Attempted to read a finished stream");
8989

9090
m_generation++;
9191
VERIFY(m_stream.discard_or_error(block_ceiling(m_header.size()) - m_file_offset));
9292
m_file_offset = 0;
9393

9494
if (!m_stream.read_or_error(Bytes(&m_header, sizeof(m_header)))) {
9595
m_finished = true;
96-
return;
96+
m_stream.handle_any_error(); // clear out errors so we don't assert
97+
return Error::from_string_literal("Failed to read the header");
9798
}
9899
if (!valid()) {
99100
m_finished = true;
100-
return;
101+
return {};
101102
}
102103

103104
VERIFY(m_stream.discard_or_error(block_size - sizeof(TarFileHeader)));
105+
return {};
104106
}
105107

106108
bool TarInputStream::valid() const

Userland/Libraries/LibArchive/TarStream.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class TarFileStream : public InputStream {
3434
class TarInputStream {
3535
public:
3636
TarInputStream(InputStream&);
37-
void advance();
37+
ErrorOr<void> advance();
3838
bool finished() const { return m_finished; }
3939
bool valid() const;
4040
TarFileHeader const& header() const { return m_header; }

Userland/Utilities/tar.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
9898
return {};
9999
};
100100

101-
for (; !tar_stream.finished(); tar_stream.advance()) {
101+
while (!tar_stream.finished()) {
102102
Archive::TarFileHeader const& header = tar_stream.header();
103103

104104
// Handle meta-entries earlier to avoid consuming the file content stream.
@@ -198,6 +198,10 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
198198

199199
// Non-global headers should be cleared after every file.
200200
local_overrides.clear();
201+
202+
auto maybe_error = tar_stream.advance();
203+
if (maybe_error.is_error())
204+
return maybe_error.error();
201205
}
202206
file_stream.close();
203207

0 commit comments

Comments
 (0)