Rust: New query rust/access-after-lifetime-ended

[geoffw0]

trafficstars

New query rust/access-after-lifetime-ended, for detecting pointer dereferences after the lifetime of the pointed-to object has ended. Makes use of some existing tests that were created for rust/access-invalid-pointer (before I realized that the idea for that query needed breaking into two separate queries). Also adds quite a lot of new test cases as well.

Note that the query is currently @precision medium due to several remaining false positive results in the tests (and in MRVA results). I made some effort to fix these, but I didn't get them all, I feel it's time to get what we have merged and plan further improvements as follow-up work.

Before merging:

• [x] QL review
• [x] docs review
• [x] DCA run

[github-actions[bot]]

QHelp previews:

rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.qhelp

Access of a pointer after its lifetime has ended

Dereferencing a pointer after the lifetime of its target has ended causes undefined behavior. Memory may be corrupted, causing the program to crash or behave incorrectly, in some cases exposing the program to potential attacks.

Recommendation

When dereferencing a pointer in unsafe code, take care that the pointer is still valid at the time it is dereferenced. Code may need to be rearranged or changed to extend lifetimes. If possible, rewrite the code using safe Rust types to avoid this kind of problem altogether.

Example

In the following example, val is local to get_pointer so its lifetime ends when that function returns. However, a pointer to val is returned and dereferenced after that lifetime has ended, causing undefined behavior:

fn get_pointer() -> *const i64 {
	let val = 123;

	&val
} // lifetime of `val` ends here, the pointer becomes dangling

fn example() {
	let ptr = get_pointer();
	let dereferenced_ptr;

	// ...

	unsafe {
		dereferenced_ptr = *ptr; // BAD: dereferences `ptr` after the lifetime of `val` has ended
	}

	// ...
}

One way to fix this is to change the return type of the function from a pointer to a Box, which ensures that the value it points to remains on the heap for the lifetime of the Box itself. Note that there is no longer a need for an unsafe block as the code no longer handles pointers directly:

fn get_box() -> Box<i64> {
	let val = 123;

	Box::new(val) // copies `val` onto the heap, where it remains for the lifetime of the `Box`.
}

fn example() {
	let ptr = get_box();
	let dereferenced_ptr;

	// ...

	dereferenced_ptr = *ptr; // GOOD

	// ...
}

References

• Rust Documentation: Behavior considered undefined >> Dangling pointers.
• Rust Documentation: Module ptr - Safety.
• Massachusetts Institute of Technology: Unsafe Rust - Dereferencing a Raw Pointer.
• Common Weakness Enumeration: CWE-825.

[geoffw0]

DCA:

• 95 results for the new query; that's quite a lot...
  • confusing results that have sources in macros - fixed ✅
  • results matching self; I think this kind of thing is probably OK: match self { EnumValue(x) => &x.y }
    • but the query is @precision medium, so deferring that for now should be OK I think
• otherwise LGTM

[geoffw0]

Thanks for the review @mchammer01 , I've accepted both suggestions. :)

[geoffw0]

CI is finally passing :tada: (thanks again @redsun82 for spotting and fixing the problem here)

@hvitved would you mind approving?