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

Tuple struct's private field can be accessed outside the defining module #111220

Closed
akonradi opened this issue May 4, 2023 · 6 comments · Fixed by #111245
Closed

Tuple struct's private field can be accessed outside the defining module #111220

akonradi opened this issue May 4, 2023 · 6 comments · Fixed by #111245
Assignees
Labels
A-visibility Area: Visibility / privacy. C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@akonradi
Copy link

akonradi commented May 4, 2023

I tried this code:

mod my {
    pub struct Foo(&'static str);
}

impl AsRef<str> for my::Foo {
    fn as_ref(&self) -> &str {
        let Self(s) = self;
        s
    }
}

I expected to see this happen: compilation should fail because the trait impl is defined outside of the module for the type but refers to its private field.

Instead, this happened: this compiled successfully

Meta

This reproduces on the playground for stable Rust, version 1.69.0: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=20f12a0f682d8dd6530d75f1b5e7a8dd

@akonradi akonradi added the C-bug Category: This is a bug. label May 4, 2023
@KittyBorgX

This comment was marked as outdated.

@taiki-e
Copy link
Member

taiki-e commented May 5, 2023

This can be reproduced with cross-crate (e.i., any other crate can access the private field). This means that all encapsulations that take advantage of the field being private can be bypassed. (For example, if we change Vec to a tuple struct, the same thing as set_len could be done in safe code by abusing this bug.)

// crate a
#[derive(Default)]
pub struct A(u32);
// crate b
pub trait B {
    fn f(&self);
}
impl B for b::A {
    fn f(&self) {
        let Self(a) = self;
        println!("{}", a);
    }
}
fn main() {
    let a = b::A::default();
    a.f();
}

@rustbot label +I-unsound

Also, this can be reproduced in all versions where self_struct_ctor is stable (e.i., 1.32+). https://godbolt.org/z/dWbKP3voc


@KittyBorgX It's a matter of how you initialize Foo. If you provide a constructor for Foo, you won't get a compile error: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3e9ec99b1aa66a2f767d61bf71cab9f2

@rustbot rustbot added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 5, 2023
@oli-obk oli-obk added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label May 5, 2023
@Noratrieb Noratrieb removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label May 5, 2023
@fee1-dead fee1-dead self-assigned this May 5, 2023
@y21
Copy link
Member

y21 commented May 5, 2023

Just for completeness, turning this into a segfault without external crates/just std:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=cd936b8cd757d76d84b7307c794bbef1

@Jules-Bertholet
Copy link
Contributor

@rustbot label A-visibility

@rustbot rustbot added the A-visibility Area: Visibility / privacy. label May 8, 2023
@lcnr
Copy link
Contributor

lcnr commented May 8, 2023

this has been unsound ever since #56365 in version 1.32

https://rust.godbolt.org/z/hPzMTY435

@apiraino
Copy link
Contributor

apiraino commented May 8, 2023

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 8, 2023
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 8, 2023
fee1-dead added a commit to fee1-dead-contrib/rust that referenced this issue May 25, 2023
This fixes rust-lang#111220 by checking the privacy of tuple constructors using `Self`, so the following code now errors

```rust
mod my {
    pub struct Foo(&'static str);
}

impl AsRef<str> for my::Foo {
    fn as_ref(&self) -> &str {
        let Self(s) = self; // previously compiled, now errors correctly
        s
    }
}
```
compiler-errors added a commit to compiler-errors/rust that referenced this issue May 26, 2023
…truct-field, r=lcnr

fix for `Self` not respecting tuple Ctor privacy

This PR fixes rust-lang#111220 by checking the privacy of tuple constructors using `Self`, so the following code now errors
```rust
mod my {
    pub struct Foo(&'static str);
}

impl AsRef<str> for my::Foo {
    fn as_ref(&self) -> &str {
        let Self(s) = self; // previously compiled, now errors correctly
        s
    }
}
```
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 26, 2023
…truct-field, r=lcnr

fix for `Self` not respecting tuple Ctor privacy

This PR fixes rust-lang#111220 by checking the privacy of tuple constructors using `Self`, so the following code now errors
```rust
mod my {
    pub struct Foo(&'static str);
}

impl AsRef<str> for my::Foo {
    fn as_ref(&self) -> &str {
        let Self(s) = self; // previously compiled, now errors correctly
        s
    }
}
```
compiler-errors added a commit to compiler-errors/rust that referenced this issue May 26, 2023
…truct-field, r=lcnr

fix for `Self` not respecting tuple Ctor privacy

This PR fixes rust-lang#111220 by checking the privacy of tuple constructors using `Self`, so the following code now errors
```rust
mod my {
    pub struct Foo(&'static str);
}

impl AsRef<str> for my::Foo {
    fn as_ref(&self) -> &str {
        let Self(s) = self; // previously compiled, now errors correctly
        s
    }
}
```
bors added a commit to rust-lang-ci/rust that referenced this issue May 27, 2023
…uct-field, r=lcnr

fix for `Self` not respecting tuple Ctor privacy

This PR fixes rust-lang#111220 by checking the privacy of tuple constructors using `Self`, so the following code now errors
```rust
mod my {
    pub struct Foo(&'static str);
}

impl AsRef<str> for my::Foo {
    fn as_ref(&self) -> &str {
        let Self(s) = self; // previously compiled, now errors correctly
        s
    }
}
```
@bors bors closed this as completed in be44860 May 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-visibility Area: Visibility / privacy. C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.