# #26 Reading on uninitialized memory may cause UB

| Field   | Value |
|---------|-------|
| **State** | open |
| **Author** | @JOE1994 |
| **Created** | 2021-01-08 |
| **Updated** | 2023-04-23 |
| **URL** | https://github.com/ruuda/claxon/issues/26 |

---

Hello :crab: ,
we (Rust group @sslab-gatech) found a memory-safety/soundness issue in this crate while scanning Rust code on crates.io for potential vulnerabilities.

## Issue Description
*  `metadata::read_vorbis_comment_block()` method

https://github.com/ruuda/claxon/blob/2f053855d581cfe3b9e7cef67fed5b0b0ccc45aa/src/metadata.rs#L432-L438

https://github.com/ruuda/claxon/blob/2f053855d581cfe3b9e7cef67fed5b0b0ccc45aa/src/metadata.rs#L473-L475

* `metadata::read_application_block()` method

https://github.com/ruuda/claxon/blob/2f053855d581cfe3b9e7cef67fed5b0b0ccc45aa/src/metadata.rs#L544-L546

Methods `metadata::read_vorbis_comment_block()` & `metadata::read_application_block()` create an uninitialized buffer and passes it to user-provided `ReadBytes` implementation. This is unsound, because it allows safe Rust code to exhibit an undefined behavior (read from uninitialized memory).

[This part](https://doc.rust-lang.org/std/io/trait.Read.html#tymethod.read) from the `Read` trait documentation explains the issue:

> It is your responsibility to make sure that `buf` is initialized before calling `read`. Calling read with an uninitialized `buf` (of the kind one obtains via `MaybeUninit<T>`) is not safe, and can lead to undefined behavior.

## Suggested Fix

It is safe to zero-initialize the newly allocated `u8` buffer before `read_into()`, in order to prevent user-provided `Read` from accessing old contents of the newly allocated heap memory.

Also, there are two nightly features for handling such cases.

* https://doc.rust-lang.org/std/io/struct.Initializer.html
* https://rust-lang.github.io/rfcs/2930-read-buf.html

Thank you for checking out this issue :+1:
