[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor/master] protover: Fix memleak in Rust impl of protover_compute_for_old_tor.
commit b85436c5961d1b2168c3954a7e718e03a121473f
Author: Isis Lovecruft <isis@xxxxxxxxxxxxxx>
Date: Tue Feb 6 14:15:33 2018 +0000
protover: Fix memleak in Rust impl of protover_compute_for_old_tor.
* FIXES #25127: https://bugs.torproject.org/25127
* ADDS a new module to the Rust tor_util crate for small utilities
for working with static strings between languages.
* CHANGES the return type of protover_compute_for_old_tor to point to
immutable data.
* CHANGES the code from the previous commit to use the new static
string utilities.
---
src/rust/protover/ffi.rs | 40 ++++++++++++++-------
src/rust/protover/lib.rs | 1 +
src/rust/protover/protover.rs | 33 ++++++++---------
src/rust/tor_util/lib.rs | 2 +-
src/rust/tor_util/strings.rs | 82 +++++++++++++++++++++++++++++++++++++++++++
5 files changed, 126 insertions(+), 32 deletions(-)
diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs
index 5fefa8f7c..5519b75ea 100644
--- a/src/rust/protover/ffi.rs
+++ b/src/rust/protover/ffi.rs
@@ -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);
+
+ // 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);
+ }
- allocate_and_copy_string(&supported)
+ supported.as_ptr()
}
diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs
index 5a5dea440..fe8c0f9bb 100644
--- a/src/rust/protover/lib.rs
+++ b/src/rust/protover/lib.rs
@@ -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;
diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs
index 00fe08408..1680d3394 100644
--- a/src/rust/protover/protover.rs
+++ b/src/rust/protover/protover.rs
@@ -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)]
diff --git a/src/rust/tor_util/lib.rs b/src/rust/tor_util/lib.rs
index 42fa9d5ad..12cb3896b 100644
--- a/src/rust/tor_util/lib.rs
+++ b/src/rust/tor_util/lib.rs
@@ -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;
diff --git a/src/rust/tor_util/strings.rs b/src/rust/tor_util/strings.rs
new file mode 100644
index 000000000..9321ce4f8
--- /dev/null
+++ b/src/rust/tor_util/strings.rs
@@ -0,0 +1,82 @@
+// 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
+}
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits