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

Suggestion: Detect derive(Deserialize) on type that has methods using unsafe #5471

Closed
thomcc opened this issue Apr 15, 2020 · 1 comment · Fixed by #5493
Closed

Suggestion: Detect derive(Deserialize) on type that has methods using unsafe #5471

thomcc opened this issue Apr 15, 2020 · 1 comment · Fixed by #5493
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience.

Comments

@thomcc
Copy link
Member

thomcc commented Apr 15, 2020

Potential for false positives, but in the past I saw something like:

#[derive(Clone, ..., Deserialize)]
pub struct NamespacedName {
    // `foo/bar` stored together
    inner: Box<str>,
    // the position of the slash -- constructor ensures it's in bounds
    split: usize,
}

impl NamespacedName {
    #[inline]
	pub fn parts(&self) -> (&str, &str) {
        unsafe { 
            let ns = self.inner.get_unchecked(..self.split);
            let name = self.inner.get_unchecked((self.split + 1)..);
            (ns, name)
        }
    }
}

Of course, while the constructor did ensure it was in bounds, serde added a second constructor which did not. I suspect other code could have a similar problem.

@flip1995 flip1995 added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. A-lint Area: New lints labels Apr 15, 2020
@ebroto
Copy link
Member

ebroto commented Apr 17, 2020

I would like to work on this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants