LibArchive: Make get_field_as_integral error out on non-octal input

Fixes this bug that was reported by OSS-Fuzz:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=52493

Userland/Libraries/LibArchive/Tar.h

@@ -51,14 +51,15 @@ constexpr StringView posix1_tar_magic = ""sv;   // POSIX.1-1988 format magic
 constexpr StringView posix1_tar_version = ""sv; // POSIX.1-1988 format version
 
 template<size_t N>
-static size_t get_field_as_integral(char const (&field)[N])
+static ErrorOr<size_t> get_field_as_integral(char const (&field)[N])
 {
     size_t value = 0;
     for (size_t i = 0; i < N; ++i) {
         if (field[i] == 0 || field[i] == ' ')
             break;
 
-        VERIFY(field[i] >= '0' && field[i] <= '7');
+        if (field[i] < '0' || field[i] > '7')
+            return Error::from_string_literal("Passed a non-octal value");
         value *= 8;
         value += field[i] - '0';
     }
@@ -89,21 +90,21 @@ static void set_octal_field(char (&field)[N], TSource&& source)
 class [[gnu::packed]] TarFileHeader {
 public:
     StringView filename() const { return get_field_as_string_view(m_filename); }
-    mode_t mode() const { return get_field_as_integral(m_mode); }
+    ErrorOr<mode_t> mode() const { return TRY(get_field_as_integral(m_mode)); }
-    uid_t uid() const { return get_field_as_integral(m_uid); }
+    ErrorOr<uid_t> uid() const { return TRY(get_field_as_integral(m_uid)); }
-    gid_t gid() const { return get_field_as_integral(m_gid); }
+    ErrorOr<gid_t> gid() const { return TRY(get_field_as_integral(m_gid)); }
     // FIXME: support 2001-star size encoding
-    size_t size() const { return get_field_as_integral(m_size); }
+    ErrorOr<size_t> size() const { return TRY(get_field_as_integral(m_size)); }
-    time_t timestamp() const { return get_field_as_integral(m_timestamp); }
+    ErrorOr<time_t> timestamp() const { return TRY(get_field_as_integral(m_timestamp)); }
-    unsigned checksum() const { return get_field_as_integral(m_checksum); }
+    ErrorOr<unsigned> checksum() const { return TRY(get_field_as_integral(m_checksum)); }
     TarFileType type_flag() const { return TarFileType(m_type_flag); }
     StringView link_name() const { return { m_link_name, strlen(m_link_name) }; }
     StringView magic() const { return get_field_as_string_view(m_magic); }
     StringView version() const { return get_field_as_string_view(m_version); }
     StringView owner_name() const { return get_field_as_string_view(m_owner_name); }
     StringView group_name() const { return get_field_as_string_view(m_group_name); }
-    int major() const { return get_field_as_integral(m_major); }
+    ErrorOr<int> major() const { return TRY(get_field_as_integral(m_major)); }
-    int minor() const { return get_field_as_integral(m_minor); }
+    ErrorOr<int> minor() const { return TRY(get_field_as_integral(m_minor)); }
     // FIXME: support ustar filename prefix
     StringView prefix() const { return get_field_as_string_view(m_prefix); }
 

Userland/Libraries/LibArchive/TarStream.cpp

@@ -25,7 +25,12 @@ size_t TarFileStream::read(Bytes bytes)
     if (has_any_error())
         return 0;
 
-    auto to_read = min(bytes.size(), m_tar_stream.header().size() - m_tar_stream.m_file_offset);
+    auto header_size_or_error = m_tar_stream.header().size();
+    if (header_size_or_error.is_error())
+        return 0;
+    auto header_size = header_size_or_error.release_value();
+
+    auto to_read = min(bytes.size(), header_size - m_tar_stream.m_file_offset);
 
     auto nread = m_tar_stream.m_stream.read(bytes.trim(to_read));
     m_tar_stream.m_file_offset += nread;
@@ -37,8 +42,13 @@ bool TarFileStream::unreliable_eof() const
     // verify that the stream has not advanced
     VERIFY(m_tar_stream.m_generation == m_generation);
 
+    auto header_size_or_error = m_tar_stream.header().size();
+    if (header_size_or_error.is_error())
+        return true;
+    auto header_size = header_size_or_error.release_value();
+
     return m_tar_stream.m_stream.unreliable_eof()
-        || m_tar_stream.m_file_offset >= m_tar_stream.header().size();
+        || m_tar_stream.m_file_offset >= header_size;
 }
 
 bool TarFileStream::read_or_error(Bytes bytes)
@@ -59,7 +69,12 @@ bool TarFileStream::discard_or_error(size_t count)
     // verify that the stream has not advanced
     VERIFY(m_tar_stream.m_generation == m_generation);
 
-    if (count > m_tar_stream.header().size() - m_tar_stream.m_file_offset) {
+    auto header_size_or_error = m_tar_stream.header().size();
+    if (header_size_or_error.is_error())
+        return false;
+    auto header_size = header_size_or_error.release_value();
+
+    if (count > header_size - m_tar_stream.m_file_offset) {
         return false;
     }
     m_tar_stream.m_file_offset += count;
@@ -88,7 +103,13 @@ ErrorOr<void> TarInputStream::advance()
         return Error::from_string_literal("Attempted to read a finished stream");
 
     m_generation++;
-    VERIFY(m_stream.discard_or_error(block_ceiling(m_header.size()) - m_file_offset));
+
+    auto header_size_or_error = m_header.size();
+    if (header_size_or_error.is_error())
+        return header_size_or_error.release_error();
+    auto header_size = header_size_or_error.release_value();
+
+    VERIFY(m_stream.discard_or_error(block_ceiling(header_size) - m_file_offset));
     m_file_offset = 0;
 
     if (!m_stream.read_or_error(Bytes(&m_header, sizeof(m_header)))) {
@@ -116,7 +137,10 @@ bool TarInputStream::valid() const
         return false;
 
     // POSIX.1-1988 tar does not have magic numbers, so we also need to verify the header checksum.
-    return header().checksum() == header().expected_checksum();
+    if (header().checksum().is_error())
+        return false;
+
+    return header().checksum().release_value() == header().expected_checksum();
 }
 
 TarFileStream TarInputStream::file_contents()

Userland/Libraries/LibArchive/TarStream.h

@@ -75,8 +75,13 @@ inline ErrorOr<void> TarInputStream::for_each_extended_header(F func)
 
     Archive::TarFileStream file_stream = file_contents();
 
-    ByteBuffer file_contents_buffer = TRY(ByteBuffer::create_zeroed(header().size()));
+    auto header_size_or_error = header().size();
-    VERIFY(file_stream.read(file_contents_buffer) == header().size());
+    if (header_size_or_error.is_error())
+        return header_size_or_error.release_error();
+    auto header_size = header_size_or_error.release_value();
+
+    ByteBuffer file_contents_buffer = TRY(ByteBuffer::create_zeroed(header_size));
+    VERIFY(file_stream.read(file_contents_buffer) == header_size);
 
     StringView file_contents { file_contents_buffer };
 

Userland/Utilities/tar.cpp

@@ -160,12 +160,17 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
                 String absolute_path = Core::File::absolute_path(filename);
                 auto parent_path = LexicalPath(absolute_path).parent();
 
+                auto header_mode_or_error = header.mode();
+                if (header_mode_or_error.is_error())
+                    return header_mode_or_error.release_error();
+                auto header_mode = header_mode_or_error.release_value();
+
                 switch (header.type_flag()) {
                 case Archive::TarFileType::NormalFile:
                 case Archive::TarFileType::AlternateNormalFile: {
                     MUST(Core::Directory::create(parent_path, Core::Directory::CreateDirectories::Yes));
 
-                    int fd = TRY(Core::System::open(absolute_path, O_CREAT | O_WRONLY, header.mode()));
+                    int fd = TRY(Core::System::open(absolute_path, O_CREAT | O_WRONLY, header_mode));
 
                     Array<u8, buffer_size> buffer;
                     size_t bytes_read;
@@ -184,7 +189,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
                 case Archive::TarFileType::Directory: {
                     MUST(Core::Directory::create(parent_path, Core::Directory::CreateDirectories::Yes));
 
-                    auto result_or_error = Core::System::mkdir(absolute_path, header.mode());
+                    auto result_or_error = Core::System::mkdir(absolute_path, header_mode);
                     if (result_or_error.is_error() && result_or_error.error().code() != EEXIST)
                         return result_or_error.error();
                     break;