sudo-rs' first security audit

sudo-rs is a project by Prossimo jointly implemented by Ferrous Systems and Tweede golf.

This article was originally written and posted by Jorge Aparicio from Ferrous Systems.

It is part of a series about sudo-rs:

• Re-implementing Sudo in Rust (when you want more than just a sandwich) by Marc.
• Testing sudo-rs and improving sudo along the way by Jorge.
• sudo-rs' first security audit by Jorge. (You are here...)

ROS performed crystal-box penetration testing on sudo-rs with the goal of verifying that it was not possible to perform privileged actions without proper authentication. The audit was carried out between September 4 and September 15 on revision b5eb2c6 of the sudo-rs codebase.

Overview of the findings

The ROS team found one moderate severity issue and two low severity issues:

• CLN-001: relative path traversal vulnerability (moderate)
• CLN-003: Cargo configuration does not strip symbols (low)
• CLN-004: incorrect default permissions on chown call (low)

In addition to these findings, ROS performed fuzzing on different components of the sudo-rs code base but found no issues. You can read the full report here.

CLN-001: relative path traversal vulnerability (moderate)

Background information

To save the user from re-entering their password on every invocation within a terminal session, sudo maintains a timestamp file that records when the last successful authentication occurred. The timestamp files for a user are stored in the directory /var/run/sudo-rs/ts/$USERNAME.

The command sudo --remove-timestamp removes all timestamp files associated with the invoking user by removing the entire /var/run/sudo-rs/ts/$USERNAME directory.

Attack vector

An attacker able to use sudo and create users with names that contain special characters like dots (.) and slashes (/) can, for example, create a user named ../../../../usr/bin/cp and then invoke sudo -K as that user to remove the /usr/bin/cp command.

sudo-rs team response

During the audit, it came to light that the original sudo implementation was also affected by this issue, although with a lower security severity due to their use of the openat function.

The team took the following actions:

• The team informed Todd Miller, the maintainer of the original sudo implementation, about the issue.
• The team prepared a fix and requested a CVE number.
• The team notified packagers of sudo-rs about the vulnerability and the bug fix release plan.
• On September 21, the team lifted the embargo on CVE-2023-42456 and released bug fix release v0.2.1

Release 1.9.15 of the original sudo will include a similar fix. There's no CVE associated to the issue in the original sudo.

CLN-003: release configuration does not strip symbols (low)

To quote ROS:

Description: The cargo release build does not strip symbols, so they will be included in the final binary. (..) Impact: Since the code is open source, there is not much information to be gained, but removing these symbols might make reverse engineering of the binary harder.

Our comments:

We consider this to be a non-issue in practice given that sudo-rs is an open source project. Regardless, we decided to modify the release profile to strip symbols in release builds.

The main reason we kept symbols in the release build was to run cargo-bloat as part of our CI checks. cargo-bloat was useful during early development as it helped us identify which external dependencies were worthwhile to remove to decrease binary size. Fast forward to today, sudo-rs only has 3 dependencies in its dependency graph and plenty of work has gone into reducing binary size so cargo-bloat has lost its relevance and we were OK to part with it.

CLN-004: incorrect default permissions on chown call (low)

To quote ROS:

A function used to invoke the libc chown function uses a default user ID value instead of failing.

The code in question was:

type UserId = u16;
type GroupId = u16;

pub fn chown<S: AsRef<CStr>>(
    path: &S,
    uid: impl Into<Option<UserId>>,
    gid: impl Into<Option<GroupId>>,
) -> io::Result<()> {
    let path = path.as_ref().as_ptr();
    let uid = uid.into().unwrap_or(UserId::MAX);
    let gid = gid.into().unwrap_or(GroupId::MAX);

    cerr(unsafe { libc::chown(path, uid, gid) }).map(|_| ())
}

Our comments:

Indeed, this function was due for some refactoring: this chown wrapper was always called with a Some uid argument. Ultimately, the function was refactored to not take any Option arguments.

Conclusion

We would like to thank ROS for conducting the first audit on sudo-rs. We have learned a lot from it and look forward to future audits and third party reviews. As ROS put in the report: "security is an ongoing process"; we agree that even after a piece of software has achieved a desired number of features, work must continue to ensure the software is free from vulnerabilities and other defects.

We would like to use this opportunity to invite security researchers, penetration testers and the general developer community to review the sudo-rs code base. Should you find a security issue, please follow our security policy to disclose it in a responsible manner. For non-security issues, you can use our bug report template on GitHub.

Thanks, the sudo-rs team