Repository: SerenityOS/serenity
File: Userland/Utilities/unzip.cpp (C++)

Code before:
        auto decompressed_data = Compress::DeflateDecompressor::decompress_all(zip_member.compressed_data);
        if (decompressed_data.is_error()) {
            warnln("Failed decompressing file {}: {}", zip_member.name, decompressed_data.error());
            return false;
        }
        if (decompressed_data.value().size() != zip_member.uncompressed_size) {
            warnln("Failed decompressing file {}", zip_member.name);
            return false;
        }
        if (auto maybe_error = new_file->write_until_depleted(decompressed_data.value()); maybe_error.is_error()) {
            warnln("Can't write file contents in {}: {}", zip_member.name, maybe_error.release_error());
            return false;
        }
        checksum.update(decompressed_data.value());
        break;
    }
    default:
        VERIFY_NOT_REACHED();
    }

    if (adjust_modification_time(zip_member).is_error()) {
        warnln("Failed setting modification_time for file {}", zip_member.name);
        return false;
    }

    if (adjust_permissions(zip_member).is_error()) {
        warnln("Failed setting permissions for file {}", zip_member.name);
        return false;
    }

    new_file->close();

    if (checksum.digest() != zip_member.crc32) {
        warnln("Failed decompressing file {}: CRC32 mismatch", zip_member.name);
        MUST(FileSystem::remove(zip_member.name, FileSystem::RecursionMode::Disallowed));
        return false;
    }

    return true;
}

ErrorOr<int> serenity_main(Main::Arguments arguments)
{
    StringView zip_file_path;
    bool quiet { false };
    bool list_files { false };
    StringView output_directory_path;
    Vector<StringView> file_filters;

    Core::ArgsParser args_parser;
    args_parser.add_option(list_files, "Only list files in the archive", "list", 'l');

Review comment (security):
Core::File::open (on line 55) takes an optional mode_t parameter. Should we create the file with the correct permissions immediately instead of creating it with default permissions and then adjusting after?

Code after:
            warnln("Failed decompressing file {}", zip_member.name);
            return false;
        }
        if (auto maybe_error = new_file->write_until_depleted(decompressed_data.value()); maybe_error.is_error()) {
            warnln("Can't write file contents in {}: {}", zip_member.name, maybe_error.release_error());
            return false;
        }
        checksum.update(decompressed_data.value());
        break;
    }
    default:
        VERIFY_NOT_REACHED();
    }

    if (adjust_modification_time(zip_member).is_error()) {
        warnln("Failed setting modification_time for file {}", zip_member.name);
        return false;
    }

    new_file->close();

    if (checksum.digest() != zip_member.crc32) {
        warnln("Failed decompressing file {}: CRC32 mismatch", zip_member.name);
        MUST(FileSystem::remove(zip_member.name, FileSystem::RecursionMode::Disallowed));
        return false;
    }

    return true;
}

ErrorOr<int> serenity_main(Main::Arguments arguments)
{
    StringView zip_file_path;
    bool quiet { false };
    bool list_files { false };
    StringView output_directory_path;
    Vector<StringView> file_filters;

    Core::ArgsParser args_parser;
    args_parser.add_option(list_files, "Only list files in the archive", "list", 'l');
    args_parser.add_option(output_directory_path, "Directory to receive the archive content", "output-directory", 'd', "path");
    args_parser.add_option(quiet, "Be less verbose", "quiet", 'q');
    args_parser.add_positional_argument(zip_file_path, "File to unzip", "path", Core::ArgsParser::Required::Yes);
    args_parser.add_positional_argument(file_filters, "Files or filters in the archive to extract", "files", Core::ArgsParser::Required::No);
    args_parser.parse(arguments);

    struct stat st = TRY(Core::System::stat(zip_file_path));

    // FIXME: Map file chunk-by-chunk once we have mmap() with offset.
    //        This will require mapping some parts then unmapping them repeatedly,
    //        but it would be significantly faster and less syscall heavy than seek()/read() at every read.