Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unnecessary unsafety in HDR decoder #980

Closed
64 opened this issue Jun 30, 2019 · 2 comments · Fixed by #985
Closed

Unnecessary unsafety in HDR decoder #980

64 opened this issue Jun 30, 2019 · 2 comments · Fixed by #985
Labels
draft Add if this issue includes suggested code, compares interfaces, preparses wording,etc kind: bug

Comments

@64
Copy link

64 commented Jun 30, 2019

ret.set_len(pixel_count);

Vec::set_len leads to the creation of uninitialized memory which can be dropped if an error occurs during decoding. Additionally the other functions like decode_component have to be very careful to overwrite every single byte in the buffer lest uninitialized memory leak to the outside.

Is there any reason why vec![0; n] can't be used here? I doubt there would be a significant performance impact, if at all.

@HeroicKatora
Copy link
Member

It's indeed not ok, maybe worse than you'd think since the type of ret is not a Vec<u8> but a generic Vec<impl Send>. @fintelia I believe the necessary changes were part of the original #887 but dropped for the 0.21 branch since they change the interface. Now would be a good time for a rebase of those since 0.22 is delayed either way.

@HeroicKatora
Copy link
Member

HeroicKatora commented Jun 30, 2019

Indeed: this PR removes it https://github.com/image-rs/image/pull/887/files#diff-9e9af5d18e72ec657d6b3c02cf35c490 (marking as draft since implementation exists).

@HeroicKatora HeroicKatora added kind: bug draft Add if this issue includes suggested code, compares interfaces, preparses wording,etc labels Jun 30, 2019
This was referenced Jul 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
draft Add if this issue includes suggested code, compares interfaces, preparses wording,etc kind: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants