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

#[zeroize(drop)] no-op in zeroize_derive v1.1 for enums #876

Closed
daxpedda opened this issue Sep 24, 2021 · 2 comments
Closed

#[zeroize(drop)] no-op in zeroize_derive v1.1 for enums #876

daxpedda opened this issue Sep 24, 2021 · 2 comments

Comments

@daxpedda
Copy link
Contributor

daxpedda commented Sep 24, 2021

I discovered a bug where #[zeroize(drop)] doesn't generate a Drop implementation when used on enums. It seems to me that it was accidentally fixed by #847 in zeroize_derive version 1.2. The bug still exists in version 1.1.

If I'm not missing something and this bug is real, version 1.1 should probably be yanked and a report filed at the RustSec Advisory Database.

Cargo.toml:

[dependencies]
zeroize = { version = "=1.4.2", features = ["zeroize_derive"] }
zeroize_derive = "=1.1"

lib.rs:

use zeroize::Zeroize;

#[derive(Zeroize)]
#[zeroize(drop)]
pub struct Works {
    field: Vec<u8>,
}

// This doesn't compile, because a conflicting implementation generated by `#[zeroize(drop)]`.
impl Drop for Works {
    fn drop(&mut self) {
        todo!()
    }
}

#[derive(Zeroize)]
#[zeroize(drop)]
pub enum Fails {
    Variant(Vec<u8>),
}

// This does compile with zeroize_derive version 1.1, meaning `#[zeroize(drop)]` didn't implement `Drop`.
impl Drop for Fails {
    fn drop(&mut self) {
        todo!()
    }
}
@tony-iqlusion
Copy link
Member

Confirmed via cargo expand. Thanks for the report.

Input:

#[derive(Zeroize)]
#[zeroize(drop)]
pub enum Foo {
    A(u8),
    B(u16),
    C(u32)
}

zeroize_derive v1.1 output:

#[allow(non_upper_case_globals)]
#[doc(hidden)]
const _DERIVE_zeroize_Zeroize_FOR_Foo: () = {
    extern crate zeroize;
    impl zeroize::Zeroize for Foo {
        fn zeroize(&mut self) {
            match self {
                Foo::A(ref mut __binding_0) => {
                    __binding_0.zeroize();
                }
                Foo::B(ref mut __binding_0) => {
                    __binding_0.zeroize();
                }
                Foo::C(ref mut __binding_0) => {
                    __binding_0.zeroize();
                }
            }
        }
    }
};

zeroize_derive v1.2 output:

#[allow(non_upper_case_globals)]
#[doc(hidden)]
const _DERIVE_zeroize_Zeroize_FOR_Foo: () = {
    extern crate zeroize;
    impl zeroize::Zeroize for Foo {
        fn zeroize(&mut self) {
            match self {
                Foo::A(ref mut __binding_0) => {
                    __binding_0.zeroize();
                }
                Foo::B(ref mut __binding_0) => {
                    __binding_0.zeroize();
                }
                Foo::C(ref mut __binding_0) => {
                    __binding_0.zeroize();
                }
            }
        }
    }
};
#[doc(hidden)]
#[allow(non_upper_case_globals)]
const _DERIVE_Drop_FOR_Foo: () = {
    impl Drop for Foo {
        fn drop(&mut self) {
            self.zeroize();
        }
    }
};

I have yanked all of the releases of zeroize_derive prior to v1.2.0.

@tony-iqlusion
Copy link
Member

Assigned RUSTSEC-2021-0115

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants