Rust: New query for bad 'ctor' initialization

[geoffw0]

New (requested) query for bad ctor initialization. See the .qhelp for an explanation of what this is. Note three limitations:

• results are perhaps more likely to be more about correctness / reliability than security (in my opinion). There is a suitable CWE though.
• no transitive results through calls (yet), because we don't have call target resolution (yet, as far as I'm aware).
• limited testing, because we don't yet have any real-world databases that use #[ctor] / #[dtor].

TODO:

• code review
• docs review

[github-actions]

QHelp previews:

rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp

Bad 'ctor' initialization

Calling functions and methods in the Rust std library from a #[ctor] or #[dtor] function is not safe. This is because the std library only guarantees stability and portability between the beginning and the end of main, whereas #[ctor] functions are called before main, and #[dtor] functions are called after it.

Recommendation

Do not call any part of the std library from a #[ctor] or #[dtor] function. Instead either:

• Move the code to a different location, such as inside your program's main function.
• Rewrite the code using an alternative library.

Example

In the following example, a #[ctor] function uses the println! macro which calls std library functions. This may cause unexpected behavior at runtime.

#[ctor::ctor]
fn bad_example() {
    println!("Hello, world!"); // BAD: the println! macro calls std library functions
}

The issue can be fixed by replacing println! with something that does not rely on the std library. In the fixed code below, we used the libc_println! macro from the libc-print library:

#[ctor::ctor]
fn good_example() {
    libc_print::libc_println!("Hello, world!"); // GOOD: libc-print does not use the std library
}

References

• GitHub: rust-ctor - Warnings.
• Rust Programming Language: Crate std - Use before and after main().
• Common Weakness Enumeration: CWE-696.
• Common Weakness Enumeration: CWE-665.

[paldepind]

results are perhaps more likely to be more about correctness / reliability than security (in my opinion). There is a suitable CWE though.

Sounds right to me.

no transitive results through calls (yet), because we don't have call target resolution (yet, as far as I'm aware).

For global data flow we use the resolved paths that the extractor gives. But I don't think we have a suitable API for using that in queries yet. In C we have getTarget on function calls. Is that the thing that we'd want for queries?

[paldepind]

Looks good to me 😄

[geoffw0]

In C we have getTarget on function calls. Is that the thing that we'd want for queries?

Yep - or getStaticTarget() in Swift. @andersfugmann pointed out that even a placeholder predicate would allow us to fill the gaps in this query implementation.

[mchammer01]

LGTM 💖
Left some comments and suggestions for your consideration (but these are minor, and I didn't want to block this PR).

[geoffw0]

Thanks for the docs review, all changes accepted.

[mchammer01]

Thanks for looking at my comments/suggestions @geoffw0 🙇🏻
LGTM