From b3d1a05261b62cb880963e5c3b3d9cdf57146eb4 Mon Sep 17 00:00:00 2001 From: asynts Date: Wed, 5 Aug 2020 14:09:38 +0200 Subject: Refactor: Expose const_cast by removing ByteBuffer::warp(const void*, size_t) This function did a const_cast internally which made the call side look "safe". This method is removed completely and call sites are replaced with ByteBuffer::wrap(const_cast(data), size) which makes the behaviour obvious. --- AK/ByteBuffer.h | 12 ++++-------- Kernel/FileSystem/ProcFS.cpp | 7 +++---- Libraries/LibELF/Image.h | 10 +++++----- Libraries/LibGemini/GeminiJob.cpp | 4 +++- Libraries/LibGfx/BMPLoader.cpp | 2 +- Libraries/LibGfx/GIFLoader.cpp | 6 +++--- Libraries/LibGfx/ICOLoader.cpp | 8 ++++---- Libraries/LibGfx/JPGLoader.cpp | 2 +- Libraries/LibGfx/PNGLoader.cpp | 2 +- Libraries/LibHTTP/HttpsJob.cpp | 5 ++++- Libraries/LibTLS/Handshake.cpp | 2 +- Libraries/LibTLS/Record.cpp | 2 +- Libraries/LibTLS/TLSv12.cpp | 2 +- Libraries/LibWeb/Loader/ResourceLoader.cpp | 2 +- Userland/test-crypto.cpp | 12 +++++++++--- 15 files changed, 42 insertions(+), 36 deletions(-) diff --git a/AK/ByteBuffer.h b/AK/ByteBuffer.h index db528f95c0..81da296d94 100644 --- a/AK/ByteBuffer.h +++ b/AK/ByteBuffer.h @@ -42,7 +42,6 @@ public: static NonnullRefPtr create_zeroed(size_t); static NonnullRefPtr copy(const void*, size_t); static NonnullRefPtr wrap(void*, size_t); - static NonnullRefPtr wrap(const void*, size_t); static NonnullRefPtr adopt(void*, size_t); ~ByteBufferImpl() { clear(); } @@ -135,7 +134,9 @@ public: static ByteBuffer create_uninitialized(size_t size) { return ByteBuffer(ByteBufferImpl::create_uninitialized(size)); } static ByteBuffer create_zeroed(size_t size) { return ByteBuffer(ByteBufferImpl::create_zeroed(size)); } static ByteBuffer copy(const void* data, size_t size) { return ByteBuffer(ByteBufferImpl::copy(data, size)); } - static ByteBuffer wrap(const void* data, size_t size) { return ByteBuffer(ByteBufferImpl::wrap(data, size)); } + // The const version of this method was removed because it was misleading, suggesting copy on write + // functionality. If you really need the old behaviour, call ByteBuffer::wrap(const_cast(data), size) + // manually. Writing to such a byte buffer invokes undefined behaviour. static ByteBuffer wrap(void* data, size_t size) { return ByteBuffer(ByteBufferImpl::wrap(data, size)); } static ByteBuffer adopt(void* data, size_t size) { return ByteBuffer(ByteBufferImpl::adopt(data, size)); } @@ -193,7 +194,7 @@ public: // I cannot hand you a slice I don't have ASSERT(offset + size <= this->size()); - return wrap(offset_pointer(offset), size); + return wrap(const_cast(offset_pointer(offset)), size); } ByteBuffer slice(size_t offset, size_t size) const @@ -302,11 +303,6 @@ inline NonnullRefPtr ByteBufferImpl::wrap(void* data, size_t siz return ::adopt(*new ByteBufferImpl(data, size, Wrap)); } -inline NonnullRefPtr ByteBufferImpl::wrap(const void* data, size_t size) -{ - return ::adopt(*new ByteBufferImpl(const_cast(data), size, Wrap)); -} - inline NonnullRefPtr ByteBufferImpl::adopt(void* data, size_t size) { return ::adopt(*new ByteBufferImpl(data, size, Adopt)); diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index 99bb40ee3a..33fda8a8c8 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -315,7 +315,7 @@ Optional procfs$pid_vm(InodeIdentifier identifier) region_object.add("cow_pages", region.cow_pages()); region_object.add("name", region.name()); region_object.add("vmobject", region.vmobject().class_name()); - + StringBuilder pagemap_builder; for (size_t i = 0; i < region.page_count(); ++i) { auto* page = region.physical_page(i); @@ -736,8 +736,7 @@ Optional procfs$cpuinfo(InodeIdentifier) KBufferBuilder builder; JsonArraySerializer array { builder }; Processor::for_each( - [&](Processor& proc) -> IterationDecision - { + [&](Processor& proc) -> IterationDecision { auto& info = proc.info(); auto obj = array.add_object(); JsonArray features; @@ -1399,7 +1398,7 @@ ssize_t ProcFSInode::write_bytes(off_t offset, ssize_t size, const u8* buffer, F ASSERT(is_persistent_inode(identifier())); // FIXME: Being able to write into ProcFS at a non-zero offset seems like something we should maybe support.. ASSERT(offset == 0); - bool success = (*write_callback)(identifier(), ByteBuffer::wrap(buffer, size)); + bool success = (*write_callback)(identifier(), ByteBuffer::wrap(const_cast(buffer), size)); ASSERT(success); return 0; } diff --git a/Libraries/LibELF/Image.h b/Libraries/LibELF/Image.h index 1291bea571..3f43377924 100644 --- a/Libraries/LibELF/Image.h +++ b/Libraries/LibELF/Image.h @@ -66,7 +66,7 @@ public: { } - ~Symbol() {} + ~Symbol() { } StringView name() const { return m_image.table_string(m_sym.st_name); } unsigned section_index() const { return m_sym.st_shndx; } @@ -92,7 +92,7 @@ public: , m_program_header_index(program_header_index) { } - ~ProgramHeader() {} + ~ProgramHeader() { } unsigned index() const { return m_program_header_index; } u32 type() const { return m_program_header.p_type; } @@ -122,7 +122,7 @@ public: , m_section_index(sectionIndex) { } - ~Section() {} + ~Section() { } StringView name() const { return m_image.section_header_table_string(m_section_header.sh_name); } unsigned type() const { return m_section_header.sh_type; } @@ -132,7 +132,7 @@ public: unsigned entry_count() const { return !entry_size() ? 0 : size() / entry_size(); } u32 address() const { return m_section_header.sh_addr; } const char* raw_data() const { return m_image.raw_data(m_section_header.sh_offset); } - ByteBuffer wrapping_byte_buffer() { return ByteBuffer::wrap(reinterpret_cast(raw_data()), size()); } + ByteBuffer wrapping_byte_buffer() { return ByteBuffer::wrap(const_cast(raw_data()), size()); } bool is_undefined() const { return m_section_index == SHN_UNDEF; } const RelocationSection relocations() const; u32 flags() const { return m_section_header.sh_flags; } @@ -166,7 +166,7 @@ public: { } - ~Relocation() {} + ~Relocation() { } unsigned offset() const { return m_rel.r_offset; } unsigned type() const { return ELF32_R_TYPE(m_rel.r_info); } diff --git a/Libraries/LibGemini/GeminiJob.cpp b/Libraries/LibGemini/GeminiJob.cpp index 91cb58cd2f..dd5a27dbc4 100644 --- a/Libraries/LibGemini/GeminiJob.cpp +++ b/Libraries/LibGemini/GeminiJob.cpp @@ -95,7 +95,9 @@ void GeminiJob::read_while_data_available(Function read) void GeminiJob::set_certificate(String certificate, String private_key) { - if (!m_socket->add_client_key(ByteBuffer::wrap(certificate.characters(), certificate.length()), ByteBuffer::wrap(private_key.characters(), private_key.length()))) { + if (!m_socket->add_client_key( + ByteBuffer::wrap(const_cast(certificate.characters()), certificate.length()), + ByteBuffer::wrap(const_cast(private_key.characters()), private_key.length()))) { dbg() << "LibGemini: Failed to set a client certificate"; // FIXME: Do something about this failure ASSERT_NOT_REACHED(); diff --git a/Libraries/LibGfx/BMPLoader.cpp b/Libraries/LibGfx/BMPLoader.cpp index a6c82ca15d..2a9dde5ff0 100644 --- a/Libraries/LibGfx/BMPLoader.cpp +++ b/Libraries/LibGfx/BMPLoader.cpp @@ -1104,7 +1104,7 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context) return false; } - auto buffer = ByteBuffer::wrap(context.data + context.data_offset, context.data_size); + auto buffer = ByteBuffer::wrap(const_cast(context.data + context.data_offset), context.data_size); if (context.dib.info.compression == Compression::RLE4 || context.dib.info.compression == Compression::RLE8 || context.dib.info.compression == Compression::RLE24) { diff --git a/Libraries/LibGfx/GIFLoader.cpp b/Libraries/LibGfx/GIFLoader.cpp index a7e3f89407..471b258a17 100644 --- a/Libraries/LibGfx/GIFLoader.cpp +++ b/Libraries/LibGfx/GIFLoader.cpp @@ -345,7 +345,7 @@ bool load_gif_frame_descriptors(GIFLoadingContext& context) if (context.data_size < 32) return false; - auto buffer = ByteBuffer::wrap(context.data, context.data_size); + auto buffer = ByteBuffer::wrap(const_cast(context.data), context.data_size); BufferStream stream(buffer); Optional format = decode_gif_header(stream); @@ -546,7 +546,7 @@ GIFImageDecoderPlugin::GIFImageDecoderPlugin(const u8* data, size_t size) m_context->data_size = size; } -GIFImageDecoderPlugin::~GIFImageDecoderPlugin() {} +GIFImageDecoderPlugin::~GIFImageDecoderPlugin() { } IntSize GIFImageDecoderPlugin::size() { @@ -591,7 +591,7 @@ bool GIFImageDecoderPlugin::set_nonvolatile() bool GIFImageDecoderPlugin::sniff() { - auto buffer = ByteBuffer::wrap(m_context->data, m_context->data_size); + auto buffer = ByteBuffer::wrap(const_cast(m_context->data), m_context->data_size); BufferStream stream(buffer); return decode_gif_header(stream).has_value(); } diff --git a/Libraries/LibGfx/ICOLoader.cpp b/Libraries/LibGfx/ICOLoader.cpp index 3026ce0f9d..8a93d49d0d 100644 --- a/Libraries/LibGfx/ICOLoader.cpp +++ b/Libraries/LibGfx/ICOLoader.cpp @@ -170,7 +170,7 @@ static size_t find_largest_image(const ICOLoadingContext& context) size_t max_area = 0; size_t index = 0; size_t largest_index = 0; - for(const auto& desc : context.images) { + for (const auto& desc : context.images) { if (desc.width * desc.height > max_area) { max_area = desc.width * desc.height; largest_index = index; @@ -182,7 +182,7 @@ static size_t find_largest_image(const ICOLoadingContext& context) static bool load_ico_directory(ICOLoadingContext& context) { - auto buffer = ByteBuffer::wrap(context.data, context.data_size); + auto buffer = ByteBuffer::wrap(const_cast(context.data), context.data_size); auto stream = BufferStream(buffer); auto image_count = decode_ico_header(stream); if (!image_count.has_value() || image_count.value() == 0) { @@ -280,7 +280,7 @@ static bool load_ico_bmp(ICOLoadingContext& context, ImageDescriptor& desc) } // Mask is 1bpp, and each row must be 4-byte aligned - size_t mask_row_len = align_up_to(align_up_to(desc.width, 8)/8, 4); + size_t mask_row_len = align_up_to(align_up_to(desc.width, 8) / 8, 4); size_t required_len = desc.height * (desc.width * sizeof(BMP_ARGB) + mask_row_len); size_t available_len = desc.size - sizeof(info); if (required_len > available_len) { @@ -408,7 +408,7 @@ bool ICOImageDecoderPlugin::set_nonvolatile() bool ICOImageDecoderPlugin::sniff() { - auto buffer = ByteBuffer::wrap(m_context->data, m_context->data_size); + auto buffer = ByteBuffer::wrap(const_cast(m_context->data), m_context->data_size); BufferStream stream(buffer); return decode_ico_header(stream).has_value(); } diff --git a/Libraries/LibGfx/JPGLoader.cpp b/Libraries/LibGfx/JPGLoader.cpp index 57772534d2..122f3ee17c 100644 --- a/Libraries/LibGfx/JPGLoader.cpp +++ b/Libraries/LibGfx/JPGLoader.cpp @@ -1146,7 +1146,7 @@ static bool scan_huffman_stream(BufferStream& stream, JPGLoadingContext& context static bool decode_jpg(JPGLoadingContext& context) { - ByteBuffer buffer = ByteBuffer::wrap(context.data, context.data_size); + ByteBuffer buffer = ByteBuffer::wrap(const_cast(context.data), context.data_size); BufferStream stream(buffer); if (!parse_header(stream, context)) return false; diff --git a/Libraries/LibGfx/PNGLoader.cpp b/Libraries/LibGfx/PNGLoader.cpp index e732ac2e24..47f0ff9994 100644 --- a/Libraries/LibGfx/PNGLoader.cpp +++ b/Libraries/LibGfx/PNGLoader.cpp @@ -166,7 +166,7 @@ public: { if (m_size_remaining < count) return false; - buffer = ByteBuffer::wrap(m_data_ptr, count); + buffer = ByteBuffer::wrap(const_cast(m_data_ptr), count); m_data_ptr += count; m_size_remaining -= count; return true; diff --git a/Libraries/LibHTTP/HttpsJob.cpp b/Libraries/LibHTTP/HttpsJob.cpp index 8b5ae352b0..29cd0ac39c 100644 --- a/Libraries/LibHTTP/HttpsJob.cpp +++ b/Libraries/LibHTTP/HttpsJob.cpp @@ -88,7 +88,10 @@ void HttpsJob::shutdown() void HttpsJob::set_certificate(String certificate, String private_key) { - if (!m_socket->add_client_key(ByteBuffer::wrap(certificate.characters(), certificate.length()), ByteBuffer::wrap(private_key.characters(), private_key.length()))) { + if (!m_socket->add_client_key( + ByteBuffer::wrap(const_cast(certificate.characters()), certificate.length()), + ByteBuffer::wrap(const_cast(private_key.characters()), private_key.length()))) { + dbg() << "LibHTTP: Failed to set a client certificate"; // FIXME: Do something about this failure ASSERT_NOT_REACHED(); diff --git a/Libraries/LibTLS/Handshake.cpp b/Libraries/LibTLS/Handshake.cpp index 99165fec5b..5cafe259e8 100644 --- a/Libraries/LibTLS/Handshake.cpp +++ b/Libraries/LibTLS/Handshake.cpp @@ -157,7 +157,7 @@ ByteBuffer TLSv12::build_finished() auto dummy = ByteBuffer::create_zeroed(0); auto digest = m_context.handshake_hash.digest(); - auto hashbuf = ByteBuffer::wrap(digest.immutable_data(), m_context.handshake_hash.digest_size()); + auto hashbuf = ByteBuffer::wrap(const_cast(digest.immutable_data()), m_context.handshake_hash.digest_size()); pseudorandom_function(outbuffer, m_context.master_key, (const u8*)"client finished", 15, hashbuf, dummy); builder.append(outbuffer); diff --git a/Libraries/LibTLS/Record.cpp b/Libraries/LibTLS/Record.cpp index 599646ff8b..8b15d465e4 100644 --- a/Libraries/LibTLS/Record.cpp +++ b/Libraries/LibTLS/Record.cpp @@ -239,7 +239,7 @@ ssize_t TLSv12::handle_message(const ByteBuffer& buffer) memcpy(temp_buf, buffer.offset_pointer(0), 3); *(u16*)(temp_buf + 3) = convert_between_host_and_network(length); auto hmac = hmac_message(ByteBuffer::wrap(temp_buf, 5), decrypted, mac_size); - auto message_mac = ByteBuffer::wrap(message_hmac, mac_size); + auto message_mac = ByteBuffer::wrap(const_cast(message_hmac), mac_size); if (hmac != message_mac) { dbg() << "integrity check failed (mac length " << length << ")"; dbg() << "mac received:"; diff --git a/Libraries/LibTLS/TLSv12.cpp b/Libraries/LibTLS/TLSv12.cpp index 5e5e401881..0579e23265 100644 --- a/Libraries/LibTLS/TLSv12.cpp +++ b/Libraries/LibTLS/TLSv12.cpp @@ -265,7 +265,7 @@ static ssize_t _parse_asn1(const Context& context, Certificate& cert, const u8* if (length == 1) cert.version = buffer[position]; } - // print_buffer(ByteBuffer::wrap(buffer + position, length)); + // print_buffer(ByteBuffer::wrap(const_cast(buffer) + position, length)); break; case 0x03: if (_asn1_is_field_present(fields, Constants::pk_id)) { diff --git a/Libraries/LibWeb/Loader/ResourceLoader.cpp b/Libraries/LibWeb/Loader/ResourceLoader.cpp index 04c2349fc1..48eff645d7 100644 --- a/Libraries/LibWeb/Loader/ResourceLoader.cpp +++ b/Libraries/LibWeb/Loader/ResourceLoader.cpp @@ -117,7 +117,7 @@ void ResourceLoader::load(const URL& url, Function(String::empty().characters()), 1), {}); }); return; } diff --git a/Userland/test-crypto.cpp b/Userland/test-crypto.cpp index 601e594eb8..36632b0a6d 100644 --- a/Userland/test-crypto.cpp +++ b/Userland/test-crypto.cpp @@ -189,12 +189,15 @@ void tls(const char* message, size_t len) void aes_cbc(const char* message, size_t len) { - auto buffer = ByteBuffer::wrap(message, len); + auto buffer = ByteBuffer::wrap(const_cast(message), len); // FIXME: Take iv as an optional parameter auto iv = ByteBuffer::create_zeroed(Crypto::Cipher::AESCipher::block_size()); if (encrypting) { - Crypto::Cipher::AESCipher::CBCMode cipher(ByteBuffer::wrap(secret_key, strlen(secret_key)), key_bits, Crypto::Cipher::Intent::Encryption); + Crypto::Cipher::AESCipher::CBCMode cipher( + ByteBuffer::wrap(const_cast(secret_key), strlen(secret_key)), + key_bits, + Crypto::Cipher::Intent::Encryption); auto enc = cipher.create_aligned_buffer(buffer.size()); (void)cipher.encrypt(buffer, enc, iv); @@ -204,7 +207,10 @@ void aes_cbc(const char* message, size_t len) else print_buffer(enc, Crypto::Cipher::AESCipher::block_size()); } else { - Crypto::Cipher::AESCipher::CBCMode cipher(ByteBuffer::wrap(secret_key, strlen(secret_key)), key_bits, Crypto::Cipher::Intent::Decryption); + Crypto::Cipher::AESCipher::CBCMode cipher( + ByteBuffer::wrap(const_cast(secret_key), strlen(secret_key)), + key_bits, + Crypto::Cipher::Intent::Decryption); auto dec = cipher.create_aligned_buffer(buffer.size()); cipher.decrypt(buffer, dec, iv); printf("%.*s\n", (int)dec.size(), dec.data()); -- cgit v1.2.3