protover: Fix memleak in Rust impl of protover_compute_for_old_tor. (b85436c5) · Commits · David Goulet / Tor

src/rust/protover/ffi.rs

+28 −12

Original line number	Diff line number	Diff line
		@@ -12,6 +12,9 @@ use std::ffi::CString;
		use protover::*;
		use smartlist::*;
		use tor_allocate::allocate_and_copy_string;
		use tor_util::strings::byte_slice_is_c_like;
		use tor_util::strings::empty_static_cstr;


		/// Translate C enums to Rust Proto enums, using the integer value of the C
		/// enum to map to its associated Rust enum
		@@ -144,8 +147,7 @@ pub extern "C" fn protover_get_supported_protocols() -> *const c_char {
		// bytes. An assert is okay here, since changing the const byte slice
		// in protover.rs to contain a NUL byte somewhere in the middle would be a
		// programming error.
		assert!(!SUPPORTED_PROTOCOLS[..SUPPORTED_PROTOCOLS.len() - 1].contains(&0x00));
		assert!(SUPPORTED_PROTOCOLS[SUPPORTED_PROTOCOLS.len() - 1] == 0x00);
		assert!(byte_slice_is_c_like(SUPPORTED_PROTOCOLS));

		// It's okay to call the "unchecked" version of the function because
		// we can see that the bytes we're passing into it 1) are valid UTF-8,
		@@ -200,15 +202,15 @@ pub extern "C" fn protover_is_supported_here(
		/// Provide an interface for C to translate arguments and return types for
		/// protover::compute_for_old_tor
		#[no_mangle]
		pub extern "C" fn protover_compute_for_old_tor(
		version: *const c_char,
		) -> *mut c_char {
		// Not handling errors when unwrapping as the content is controlled
		// and is an empty string
		let empty = String::new();
		pub extern "C" fn protover_compute_for_old_tor(version: const c_char) -> const c_char {
		let supported: &'static CStr;
		let elder_protocols: &'static [u8];
		let empty: &'static CStr;

		empty = empty_static_cstr();

		if version.is_null() {
		return allocate_and_copy_string(&empty);
		return empty.as_ptr();
		}

		// Require an unsafe block to read the version from a C string. The pointer
		@@ -217,10 +219,24 @@ pub extern "C" fn protover_compute_for_old_tor(

		let version = match c_str.to_str() {
		Ok(n) => n,
		Err(_) => return allocate_and_copy_string(&empty),
		Err(_) => return empty.as_ptr(),
		};

		let supported = compute_for_old_tor(&version);
		elder_protocols = compute_for_old_tor(&version);

		allocate_and_copy_string(&supported)
		// If we're going to pass it to C, there cannot be any intermediate NUL
		// bytes. An assert is okay here, since changing the const byte slice
		// in protover.rs to contain a NUL byte somewhere in the middle would be a
		// programming error.
		assert!(byte_slice_is_c_like(elder_protocols));

		// It's okay to call the "unchecked" version of the function because
		// we can see that the bytes we're passing into it 1) are valid UTF-8,
		// 2) have no intermediate NUL bytes, and 3) are terminated with a NUL
		// byte.
		unsafe {
		supported = CStr::from_bytes_with_nul_unchecked(elder_protocols);
		}

		supported.as_ptr()
		}

src/rust/protover/lib.rs

+1 −0

Original line number	Diff line number	Diff line
		@@ -26,6 +26,7 @@ extern crate libc;
		extern crate smartlist;
		extern crate external;
		extern crate tor_allocate;
		extern crate tor_util;

		mod protover;
		pub mod ffi;

src/rust/protover/protover.rs

+14 −19

Original line number	Diff line number	Diff line
		@@ -10,6 +10,7 @@ use std::collections::{HashMap, HashSet};
		use std::ops::Range;
		use std::string::String;

		use tor_util::strings::NUL_BYTE;

		/// The first version of Tor that included "proto" entries in its descriptors.
		/// Authorities should use this to decide whether to guess proto lines.
		@@ -724,11 +725,11 @@ pub fn is_supported_here(proto: Proto, vers: u32) -> bool {
		///
		/// # Inputs
		///
		/// * `version`, a string comprised of "[0-9,-]"
		/// * `version`, a string comprised of "[0-9a-z.-]"
		///
		/// # Returns
		///
		/// A `String` whose value is series of pairs, comprising of the protocol name
		/// A `&'static [u8]` whose value is series of pairs, comprising of the protocol name
		/// and versions that it supports. The string takes the following format:
		///
		/// "HSDir=1-1 LinkAuth=1"
		@@ -737,33 +738,27 @@ pub fn is_supported_here(proto: Proto, vers: u32) -> bool {
		/// only for tor versions older than FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS.
		///
		/// C_RUST_COUPLED: src/rust/protover.c `compute_for_old_tor`
		pub fn compute_for_old_tor(version: &str) -> String {
		if c_tor_version_as_new_as(
		version,
		FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS,
		)
		{
		return String::new();
		pub fn compute_for_old_tor(version: &str) -> &'static [u8] {
		if c_tor_version_as_new_as(version, FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS) {
		return NUL_BYTE;
		}

		if c_tor_version_as_new_as(version, "0.2.9.1-alpha") {
		let ret = "Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1-2 \
		Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2";
		return String::from(ret);
		return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1-2 \
		Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0";
		}

		if c_tor_version_as_new_as(version, "0.2.7.5") {
		let ret = "Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
		Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2";
		return String::from(ret);
		return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
		Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0";
		}

		if c_tor_version_as_new_as(version, "0.2.4.19") {
		let ret = "Cons=1 Desc=1 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
		Link=1-4 LinkAuth=1 Microdesc=1 Relay=1-2";
		return String::from(ret);
		return b"Cons=1 Desc=1 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
		Link=1-4 LinkAuth=1 Microdesc=1 Relay=1-2\0";
		}
		String::new()

		NUL_BYTE
		}

		#[cfg(test)]

src/rust/tor_util/lib.rs

+1 −1

Original line number	Diff line number	Diff line
		@@ -3,9 +3,9 @@

		//! Small module to announce Rust support during startup for demonstration
		//! purposes.
		//!

		extern crate libc;
		extern crate tor_allocate;

		pub mod ffi;
		pub mod strings;

src/rust/tor_util/strings.rs

0 → 100644

+82 −0

Original line number	Diff line number	Diff line
		// Copyright (c) 2016-2017, The Tor Project, Inc. */
		// See LICENSE for licensing information */

		//! Utilities for working with static strings.

		use std::ffi::CStr;

		/// A byte-array containing a single NUL byte (`b"\0"`).
		pub const NUL_BYTE: &'static [u8] = b"\0";

		/// Determine if a byte slice is a C-like string.
		///
		/// These checks guarantee that:
		///
		/// 1. there are no intermediate NUL bytes
		/// 2. the last byte is a NUL byte
		///
		/// # Warning
		///
		/// This function does _not_ guarantee that the bytes represent any valid
		/// encoding such as ASCII or UTF-8.
		///
		/// # Examples
		///
		/// ```
		/// # use tor_util::strings::byte_slice_is_c_like;
		/// #
		/// let bytes: &[u8] = b"foo bar baz";
		///
		/// assert!(byte_slice_is_c_like(&bytes) == false);
		///
		/// let bytes: &[u8] = b"foo\0bar baz";
		///
		/// assert!(byte_slice_is_c_like(&bytes) == false);
		///
		/// let bytes: &[u8] = b"foo bar baz\0";
		///
		/// assert!(byte_slice_is_c_like(&bytes) == true);
		/// ```
		pub fn byte_slice_is_c_like(bytes: &[u8]) -> bool {
		if !bytes[..bytes.len() - 1].contains(&0x00) && bytes[bytes.len() - 1] == 0x00 {
		return true;
		}
		false
		}

		/// Get a static `CStr` containing a single `NUL_BYTE`.
		///
		/// # Examples
		///
		/// When used as follows in a Rust FFI function, which could be called
		/// from C:
		///
		/// ```
		/// # extern crate libc;
		/// # extern crate tor_util;
		/// #
		/// # use tor_util::strings::empty_static_cstr;
		/// use libc::c_char;
		/// use std::ffi::CStr;
		///
		/// pub extern "C" fn give_c_code_an_empty_static_string() -> *const c_char {
		/// let empty: &'static CStr = empty_static_cstr();
		///
		/// empty.as_ptr()
		/// }
		///
		/// # fn main() {
		/// # give_c_code_an_empty_static_string();
		/// # }
		/// ```
		///
		/// This equates to an "empty" `const char*` static string in C.
		pub fn empty_static_cstr() -> &'static CStr {
		let empty: &'static CStr;

		unsafe {
		empty = CStr::from_bytes_with_nul_unchecked(NUL_BYTE);
		}

		empty
		}