Opened 2 years ago

Closed 13 months ago

#27616 closed task (fixed)

Double-check Rust code for potential proxy bypass in ESR 60

Reported by: gk Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: TorBrowserTeam201904
Cc: sysrqb, Actual Points:
Parent ID: #22176 Points:
Reviewer: Sponsor:


I think we are good with respect to the Rust proxy adherence situation for ESR 60 but I think another set of (fresh) eyes could not hurt, given this is a lot of code that can in theory do nasty networking stuff and we can't rip it out as we would usually do.

Child Tickets

Change History (14)

comment:1 Changed 2 years ago by gk

Parent ID: #22176

comment:2 Changed 2 years ago by gk

See the second part of comment:15:ticket:22176 for how I tried to deal with the Rust proxy audit. I hope we find some more stream-lined approach during this ESR cycle and/or make progress on and related bugs.

comment:3 Changed 2 years ago by gk

FWIW and just to make a note: geckodriver is about to get built my default: It's not getting built during esr60 and it won't get built by default if --disable-tests gets specified, which we do.

comment:4 in reply to:  2 Changed 23 months ago by sysrqb

Replying to gk:

See the second part of comment:15:ticket:22176

Okay, I started with gk's 3) from that ticket. First, I enumerated all packages and their dependencies (not including the vendored crates). From these packages, I searched for all occurrences of "tcp", "udp", "socket", "bind", "connect", "listener", "send", "recv", and "stream". (I don't claim these are the only functions/methods that can be used for transmitting a message).

I found these are the in-tree packages (not vendored in third_party/rust):


For each of those packages, I ran

$ grep -rni -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" $p

(where $p was each directory path from above).

Many of the results were false-positives. In particular, bind matched many incstances of "binding" or "bindgen". So, excluding those:

$ grep -rni -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" $p | grep -v -E "[bB]inding|[bB]indgen" | grep -ni --color=always -E "tcp|udp|socket|bind|connect|listener|send|recv|stream"

These directories didn't contain any matches:


media/mp4parse-rust/mp4parse_capi has instances of "stream" (but that's not surprising considering it's doc comment says "Parses ISO Base Media Format aka video/mp4 streams."). All instances of stream are from audio (FLAC) track information.

xpcom/rust/xpcom/xpcom_macros has a occurrence of "bind" and a few instances of "stream". "bind" is related to FFI, and "stream" are TokenStreams.

dom/webauthn/u2f-hid-rs has "send" and "recv", but these are methods called on a std::sync::mpsc::channel. There is another wrapper method sendrecv that calls U2FHIDCont::write and U2FHIDInit::read for reading/writing the U2F device. These read/write methods specifically take a device as the first argument. Using this for making network calls seems very difficult (without digging too deep).

(to be continued.)

comment:5 Changed 22 months ago by gk

Keywords: TorBrowserTeam201810 added

comment:6 Changed 22 months ago by sysrqb

Okay. Back to this.

I took a slightly different approach.

Step 1. Find all Cargo.toml files starting from the root of the repo. These will be useful next when we must find where the vendored crate is located within the repo.

$ find . -name Cargo.toml > all_cargo_toml

Step 2. Find the package name within each Cargo.toml files - these are the crate names we'll need later. These are of the form path/to/Cargo.toml:name = "name-of-crate".

$ while read crate; do echo -n $crate:; grep -A4 '\[package\]' $crate | grep 'name ='; done < all_cargo_toml | grep 'toml:name =' > all_rust_crates

Step 3. From the list of crates, from the ones currently being used (or potentially being used)

$ while read crate; do grep "= \"$crate\"" all_rust_crates; done < rust_crates | sort > used_crates

Step 4. Search the used crates for expected proxy-bypass variables/functinos/methods/etc.

$ cut -d: -f 1 used_crates | sed 's/Cargo.toml//' | xargs grep -rni -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" | grep -v -E "[bB]inding|[bB]indgen" | grep -ni --color=always -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" | less -R

This resulted in 15373 matches.

We can prevent 100 matches by excluding the directories audited in the previous comment.

$ cut -d: -f 1 used_crates | sed 's/Cargo.toml//' | xargs grep -rni -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" | grep -v -E "[bB]inding|[bB]indgen" | grep -v -e '^./media/mp4parse-rust/mp4parse_capi' -e '^./servo/support/gecko/nsstring' -e '^./xpcom/rust/nserror' -e '^./netwerk/base/rust-helper' -e '^./xpcom/rust/xpcom' -e '^./modules/libpref/parser' -e '^./netwerk/base/rust-url-capi' -e '^./dom/webauthn/u2f-hid-rs' -e '^servo/ports/geckolib' | grep -ni --color=always -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" | less -R

(to be to continued)

comment:7 Changed 22 months ago by gk

I realized that Mozilla imported a ton of updated crates and new ones for the 60.3.0esr release. I wondered whether our proposed audit strategy can cope with such a scenario as we ideally want to make sure that those updates don't include proxy bypasses. If not, it might be worth thinking about an audit strategy that *does* take such point release updates into account.

comment:8 Changed 21 months ago by gk

Keywords: TorBrowserTeam201811 added; TorBrowserTeam201810 removed

Moving our tickets to November.

comment:9 Changed 20 months ago by gk

Keywords: TorBrowserTeam201812 added; TorBrowserTeam201811 removed

Moving our tickets to December.

comment:10 Changed 19 months ago by gk

Keywords: TorBrowserTeam201901 added; TorBrowserTeam201812 removed

Moving tickets to Jan 2019.

comment:11 Changed 18 months ago by gk

Keywords: TorBrowserTeam201902 added; TorBrowserTeam201901 removed

Moving tickets to February.

comment:12 Changed 17 months ago by gk

Keywords: TorBrowserTeam201903 added; TorBrowserTeam201902 removed

Moving remaining tickets to March.

comment:13 Changed 16 months ago by gk

Keywords: TorBrowserTeam201904 added; TorBrowserTeam201903 removed

Moving tickets to April.

comment:14 Changed 13 months ago by gk

Resolution: fixed
Status: newclosed

We have kind of a double-check with tjr's work over in That needs to be enough for now, given that esr68 got just released and we need to prepare for that one.

Note: See TracTickets for help on using tickets.