Skip to content

Commit d64a77b

Browse files
committed
Add a new lint for catching unbound lifetimes in return values
This lint is based on some unsoundness found in Libra during security audits. Some function signatures can look sound, but based on the concrete types that end up being used, lifetimes may be unbound instead of anchored to the references of the arguments or the fields of a struct. When combined with unsafe code, this can cause transmuted lifetimes to be not what was intended.
1 parent 52b9e70 commit d64a77b

12 files changed

+310
-25
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,6 +1318,7 @@ Released 2018-09-13
13181318
[`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err
13191319
[`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
13201320
[`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds
1321+
[`unbound_return_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#unbound_return_lifetimes
13211322
[`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc
13221323
[`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented
13231324
[`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
88

9-
[There are 345 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
9+
[There are 346 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1010

1111
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1212

clippy_lints/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ pub mod transmuting_null;
296296
pub mod trivially_copy_pass_by_ref;
297297
pub mod try_err;
298298
pub mod types;
299+
pub mod unbound_return_lifetimes;
299300
pub mod unicode;
300301
pub mod unsafe_removed_from_name;
301302
pub mod unused_io_amount;
@@ -785,6 +786,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
785786
&types::UNIT_CMP,
786787
&types::UNNECESSARY_CAST,
787788
&types::VEC_BOX,
789+
&unbound_return_lifetimes::UNBOUND_RETURN_LIFETIMES,
788790
&unicode::NON_ASCII_LITERAL,
789791
&unicode::UNICODE_NOT_NFC,
790792
&unicode::ZERO_WIDTH_SPACE,
@@ -952,6 +954,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
952954
store.register_late_pass(|| box mul_add::MulAddCheck);
953955
store.register_late_pass(|| box mut_key::MutableKeyType);
954956
store.register_late_pass(|| box modulo_arithmetic::ModuloArithmetic);
957+
store.register_late_pass(|| box unbound_return_lifetimes::UnboundReturnLifetimes);
955958
store.register_early_pass(|| box reference::DerefAddrOf);
956959
store.register_early_pass(|| box reference::RefInDeref);
957960
store.register_early_pass(|| box double_parens::DoubleParens);
@@ -1324,6 +1327,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
13241327
LintId::of(&types::UNIT_CMP),
13251328
LintId::of(&types::UNNECESSARY_CAST),
13261329
LintId::of(&types::VEC_BOX),
1330+
LintId::of(&unbound_return_lifetimes::UNBOUND_RETURN_LIFETIMES),
13271331
LintId::of(&unicode::ZERO_WIDTH_SPACE),
13281332
LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
13291333
LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT),
@@ -1574,6 +1578,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
15741578
LintId::of(&types::CAST_PTR_ALIGNMENT),
15751579
LintId::of(&types::CAST_REF_TO_MUT),
15761580
LintId::of(&types::UNIT_CMP),
1581+
LintId::of(&unbound_return_lifetimes::UNBOUND_RETURN_LIFETIMES),
15771582
LintId::of(&unicode::ZERO_WIDTH_SPACE),
15781583
LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT),
15791584
LintId::of(&unwrap::PANICKING_UNWRAP),
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
use if_chain::if_chain;
2+
use rustc::declare_lint_pass;
3+
use rustc::hir::intravisit::*;
4+
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
5+
use rustc_hir::*;
6+
use rustc_session::declare_tool_lint;
7+
8+
use crate::utils::span_lint;
9+
10+
declare_clippy_lint! {
11+
/// **What it does:** Checks for lifetime annotations on return values
12+
/// which might not always get bound.
13+
///
14+
/// **Why is this bad?** If the function contains unsafe code which
15+
/// transmutes to the lifetime, the resulting lifetime may live longer
16+
/// than was intended.
17+
///
18+
/// **Known problems*: None.
19+
///
20+
/// **Example:**
21+
/// ```rust
22+
/// struct WrappedStr(str);
23+
///
24+
/// // Bad: unbound return lifetime causing unsoundness (e.g. when x is String)
25+
/// fn unbound<'a>(x: impl AsRef<str> + 'a) -> &'a WrappedStr {
26+
/// let s = x.as_ref();
27+
/// unsafe { &*(s as *const str as *const WrappedStr) }
28+
/// }
29+
///
30+
/// // Good: bound return lifetime is sound
31+
/// fn bound<'a>(x: &'a str) -> &'a WrappedStr {
32+
/// unsafe { &*(x as *const str as *const WrappedStr) }
33+
/// }
34+
/// ```
35+
pub UNBOUND_RETURN_LIFETIMES,
36+
correctness,
37+
"unbound lifetimes in function return values"
38+
}
39+
40+
declare_lint_pass!(UnboundReturnLifetimes => [UNBOUND_RETURN_LIFETIMES]);
41+
42+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnboundReturnLifetimes {
43+
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'tcx>) {
44+
if let ItemKind::Fn(ref sig, ref generics, _id) = item.kind {
45+
check_fn_inner(cx, &sig.decl, generics, None);
46+
}
47+
}
48+
49+
fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx ImplItem<'tcx>) {
50+
if let ImplItemKind::Method(ref sig, _) = item.kind {
51+
let parent_generics = impl_generics(cx, item.hir_id);
52+
check_fn_inner(cx, &sig.decl, &item.generics, parent_generics);
53+
}
54+
}
55+
}
56+
57+
fn check_fn_inner<'a, 'tcx>(
58+
cx: &LateContext<'a, 'tcx>,
59+
decl: &'tcx FnDecl<'tcx>,
60+
generics: &'tcx Generics<'tcx>,
61+
parent_data: Option<(&'tcx Generics<'tcx>, &'tcx Ty<'tcx>)>,
62+
) {
63+
let output_type = if let FunctionRetTy::Return(ref output_type) = decl.output {
64+
output_type
65+
} else {
66+
return;
67+
};
68+
69+
let lt = if let TyKind::Rptr(ref lt, _) = output_type.kind {
70+
lt
71+
} else {
72+
return;
73+
};
74+
75+
if matches!(lt.name, LifetimeName::Param(_)) {
76+
let target_lt = lt;
77+
78+
// check function generics
79+
// the target lifetime parameter must appear on the left of some outlives relation
80+
if lifetime_outlives_something(target_lt, generics) {
81+
return;
82+
}
83+
84+
// check parent generics
85+
// the target lifetime parameter must appear on the left of some outlives relation
86+
if let Some((ref parent_generics, _)) = parent_data {
87+
if lifetime_outlives_something(target_lt, parent_generics) {
88+
return;
89+
}
90+
}
91+
92+
// check type generics
93+
// the target lifetime parameter must be included in the type
94+
if let Some((_, ref parent_ty)) = parent_data {
95+
if TypeVisitor::type_contains_lifetime(parent_ty, target_lt) {
96+
return;
97+
}
98+
}
99+
100+
// check arguments the target lifetime parameter must be included as a
101+
// lifetime of a reference, either directly or through the gneric
102+
// parameters of the argument type.
103+
for input in decl.inputs.iter() {
104+
if TypeVisitor::type_contains_lifetime(input, target_lt) {
105+
return;
106+
}
107+
}
108+
109+
span_lint(
110+
cx,
111+
UNBOUND_RETURN_LIFETIMES,
112+
target_lt.span,
113+
"lifetime is unconstrained",
114+
);
115+
}
116+
}
117+
118+
struct TypeVisitor<'tcx> {
119+
found: bool,
120+
target_lt: &'tcx Lifetime,
121+
}
122+
123+
impl<'tcx> TypeVisitor<'tcx> {
124+
fn type_contains_lifetime(ty: &Ty<'_>, target_lt: &'tcx Lifetime) -> bool {
125+
let mut visitor = TypeVisitor {
126+
found: false,
127+
target_lt,
128+
};
129+
walk_ty(&mut visitor, ty);
130+
visitor.found
131+
}
132+
}
133+
134+
impl<'tcx> Visitor<'tcx> for TypeVisitor<'tcx> {
135+
fn visit_lifetime(&mut self, lt: &'tcx Lifetime) {
136+
if lt.name == self.target_lt.name {
137+
self.found = true;
138+
}
139+
}
140+
141+
fn visit_ty(&mut self, ty: &'tcx Ty<'_>) {
142+
match ty.kind {
143+
TyKind::Rptr(ref lt, _) => {
144+
if lt.name == self.target_lt.name {
145+
self.found = true;
146+
}
147+
},
148+
TyKind::Path(ref qpath) => {
149+
if !self.found {
150+
walk_qpath(self, qpath, ty.hir_id, ty.span);
151+
}
152+
},
153+
_ => (),
154+
}
155+
}
156+
157+
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
158+
NestedVisitorMap::None
159+
}
160+
}
161+
162+
fn lifetime_outlives_something<'tcx>(target_lt: &'tcx Lifetime, generics: &'tcx Generics<'tcx>) -> bool {
163+
if let Some(param) = generics.get_named(target_lt.name.ident().name) {
164+
if param.bounds.iter().any(|b| matches!(b, GenericBound::Outlives(_))) {
165+
return true;
166+
}
167+
}
168+
false
169+
}
170+
171+
fn impl_generics<'tcx>(cx: &LateContext<'_, 'tcx>, hir_id: HirId) -> Option<(&'tcx Generics<'tcx>, &'tcx Ty<'tcx>)> {
172+
let parent_impl = cx.tcx.hir().get_parent_item(hir_id);
173+
if_chain! {
174+
if parent_impl != CRATE_HIR_ID;
175+
if let Node::Item(item) = cx.tcx.hir().get(parent_impl);
176+
if let ItemKind::Impl(_, _, _, ref parent_generics, _, ref ty, _) = item.kind;
177+
then {
178+
return Some((parent_generics, ty))
179+
}
180+
}
181+
None
182+
}

src/lintlist/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub use lint::Lint;
66
pub use lint::LINT_LEVELS;
77

88
// begin lint list, do not remove this comment, it’s used in `update_lints`
9-
pub const ALL_LINTS: [Lint; 345] = [
9+
pub const ALL_LINTS: [Lint; 346] = [
1010
Lint {
1111
name: "absurd_extreme_comparisons",
1212
group: "correctness",
@@ -2079,6 +2079,13 @@ pub const ALL_LINTS: [Lint; 345] = [
20792079
deprecation: None,
20802080
module: "trait_bounds",
20812081
},
2082+
Lint {
2083+
name: "unbound_return_lifetimes",
2084+
group: "correctness",
2085+
desc: "unbound lifetimes in function return values",
2086+
deprecation: None,
2087+
module: "unbound_return_lifetimes",
2088+
},
20822089
Lint {
20832090
name: "unicode_not_nfc",
20842091
group: "pedantic",

tests/ui/extra_unused_lifetimes.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
dead_code,
44
clippy::needless_lifetimes,
55
clippy::needless_pass_by_value,
6-
clippy::trivially_copy_pass_by_ref
6+
clippy::trivially_copy_pass_by_ref,
7+
clippy::unbound_return_lifetimes
78
)]
89
#![warn(clippy::extra_unused_lifetimes)]
910

tests/ui/extra_unused_lifetimes.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,25 @@
11
error: this lifetime isn't used in the function definition
2-
--> $DIR/extra_unused_lifetimes.rs:14:14
2+
--> $DIR/extra_unused_lifetimes.rs:15:14
33
|
44
LL | fn unused_lt<'a>(x: u8) {}
55
| ^^
66
|
77
= note: `-D clippy::extra-unused-lifetimes` implied by `-D warnings`
88

99
error: this lifetime isn't used in the function definition
10-
--> $DIR/extra_unused_lifetimes.rs:16:25
10+
--> $DIR/extra_unused_lifetimes.rs:17:25
1111
|
1212
LL | fn unused_lt_transitive<'a, 'b: 'a>(x: &'b u8) {
1313
| ^^
1414

1515
error: this lifetime isn't used in the function definition
16-
--> $DIR/extra_unused_lifetimes.rs:41:10
16+
--> $DIR/extra_unused_lifetimes.rs:42:10
1717
|
1818
LL | fn x<'a>(&self) {}
1919
| ^^
2020

2121
error: this lifetime isn't used in the function definition
22-
--> $DIR/extra_unused_lifetimes.rs:67:22
22+
--> $DIR/extra_unused_lifetimes.rs:68:22
2323
|
2424
LL | fn unused_lt<'a>(x: u8) {}
2525
| ^^

tests/ui/needless_lifetimes.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
#![warn(clippy::needless_lifetimes)]
2-
#![allow(dead_code, clippy::needless_pass_by_value, clippy::trivially_copy_pass_by_ref)]
2+
#![allow(
3+
dead_code,
4+
clippy::needless_pass_by_value,
5+
clippy::trivially_copy_pass_by_ref,
6+
clippy::unbound_return_lifetimes
7+
)]
38

49
fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) {}
510

0 commit comments

Comments
 (0)