Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T15:52:44Zhttps://gitlab.torproject.org/legacy/trac/-/issues/33788Check the return value of tor_inet_ntop() and tor_inet_ntoa()2020-06-13T15:52:44ZteorCheck the return value of tor_inet_ntop() and tor_inet_ntoa()The following functions don't check the return value of tor_inet_ntop() or tor_inet_ntoa():
IPv6, could be serious:
* evdns_callback(), multiple times
IPv4 only, unlikely to be a serious bug:
* tor_dup_ip()
* fmt_addr32()
* evdns_wildc...The following functions don't check the return value of tor_inet_ntop() or tor_inet_ntoa():
IPv6, could be serious:
* evdns_callback(), multiple times
IPv4 only, unlikely to be a serious bug:
* tor_dup_ip()
* fmt_addr32()
* evdns_wildcard_check_callback()
These functions should log a bug log using BUG() or tor_assert_nonfatal(), and return an error. (Or for the formatting functions, a sensible placeholder string.)
We will also need to make their callers check for the error.Tor: 0.4.4.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/33284Make the pre-commit hook call the user's hooks, not the repository hooks2020-06-13T15:51:20ZteorMake the pre-commit hook call the user's hooks, not the repository hooksWe should stop executing untrusted scripts.We should stop executing untrusted scripts.Tor: 0.4.4.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/33239Prop 312: 3.2.3 Limit Directory Authority Addresses to Address and ORPort2020-06-13T15:51:06ZteorProp 312: 3.2.3 Limit Directory Authority Addresses to Address and ORPortFor security reasons, directory authorities only use addresses that are
explicitly configured in their torrc. Since local interface addresses are
implicit, and may depend on DHCP, directory authorities do not use this
address resolution ...For security reasons, directory authorities only use addresses that are
explicitly configured in their torrc. Since local interface addresses are
implicit, and may depend on DHCP, directory authorities do not use this
address resolution method (or any of the other, lower-priority address
resolution methods).
See proposal 312, section 3.2.3, directory authority case:
https://gitweb.torproject.org/torspec.git/tree/proposals/312-relay-auto-ipv6-addr.txt#n388https://gitlab.torproject.org/legacy/trac/-/issues/33237Prop 312: 3.2.2. Stop Directory Authorities Resolving *Port Hostnames2020-06-13T15:51:04ZteorProp 312: 3.2.2. Stop Directory Authorities Resolving *Port HostnamesFor security reasons, directory authorities only use addresses that are
explicitly configured in their torrc. Therefore, we propose that directory
authorities only accept IPv4 or IPv6 address literals in the address part
of the ORPort an...For security reasons, directory authorities only use addresses that are
explicitly configured in their torrc. Therefore, we propose that directory
authorities only accept IPv4 or IPv6 address literals in the address part
of the ORPort and DirPort options.
As part of this fix, we may also ban DNS resolution on all configured Ports. (We should try to avoid banning DNS resolution entirely on authorities, because some test networks use Authority/Exits.)
See proposal 312, section 3.2.2, directory authority case:
https://gitweb.torproject.org/torspec.git/tree/proposals/312-relay-auto-ipv6-addr.txt#n340
Directory authorities must not attempt to resolve these
addresses using DNS. It is a config error to provide a hostname as a
directory authority's ORPort or DirPort.
If directory authorities don't have an IPv4 address literal in their
Address or ORPort, they should issue a configuration error, and refuse to
launch. If directory authorities don't have an IPv6 address literal in their
Address or ORPort, they should issue a notice-level log, and fall back to
only using IPv4.Tor: 0.4.5.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/33093Use IF_BUG_ONCE in buf_flush_to_tls()2020-06-13T15:50:30ZNick MathewsonUse IF_BUG_ONCE in buf_flush_to_tls()See parent: the BUG() messages in this function caused dgoulet to run out of disk.See parent: the BUG() messages in this function caused dgoulet to run out of disk.Tor: 0.4.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/32637SocksPort IPv6 flags differs in default config and in Torlauncher prefs, and ...2020-06-13T15:52:10ZcypherpunksSocksPort IPv6 flags differs in default config and in Torlauncher prefs, and exits can distinguish themBy default tor daemon set only IPv6Traffic flag in client sockets, but not PreferIPv6
https://gitweb.torproject.org/tor.git/tree/src/app/config/config.c#n6094
But Torlauncher sets them both when launch tor daemon with Tor Browser
https:...By default tor daemon set only IPv6Traffic flag in client sockets, but not PreferIPv6
https://gitweb.torproject.org/tor.git/tree/src/app/config/config.c#n6094
But Torlauncher sets them both when launch tor daemon with Tor Browser
https://gitweb.torproject.org/tor-launcher.git/tree/src/defaults/preferences/torlauncher-prefs.js#n41
As i can see in spec, this flags sets bits in "FLAGS" value in RELAY_BEGIN cell, and exit relays can recognize which flags sets in client port settings. So, exits can distinguish circuits from Tor Browser Bundle clients and, as example, from daemons in linux distro repositories, with high level of probability.
https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt#n1595
Is there any reasons why PreferIPv6 flag is not turned on by default? This issue raises up in past (#21269), but solution is not efficient - anyway flags sent to exit by default differs from vast majority of real usecases - surfing web with TBB.Tor: 0.4.3.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/31507Change the client default to AvoidDiskWrites 1, or otherwise make disk writes...2020-06-13T15:44:34ZteorChange the client default to AvoidDiskWrites 1, or otherwise make disk writes less frequent.Since 2007, we've seen some significant technological changes:
* More devices with batteries
* More devices with SSDs
* Better disk forensics techniques
So it might be a good idea to change the client default to AvoidDiskWrites 1.Since 2007, we've seen some significant technological changes:
* More devices with batteries
* More devices with SSDs
* Better disk forensics techniques
So it might be a good idea to change the client default to AvoidDiskWrites 1.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/31089Consider using data-URI to embed how_tor_works_thumb.png image into tor-exit-...2020-06-13T15:43:25Zrl1987Consider using data-URI to embed how_tor_works_thumb.png image into tor-exit-notice.htmlWe can only serve a single HTML file with `DirPortFrontPage` configuration option. Currently we provide an exit notice file in tor-exit-notice.html, which embeds an image with basic Tor network schematics from Tor website. We may want to...We can only serve a single HTML file with `DirPortFrontPage` configuration option. Currently we provide an exit notice file in tor-exit-notice.html, which embeds an image with basic Tor network schematics from Tor website. We may want to use data-URI format (as described in RFC 2397) to hardcode this image into HTML and avoid loading it from external webserver.Tor: 0.2.9.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/31001Undefined behavior in tor_vasprintf()2020-06-13T15:43:04ZGeorge KadianakisUndefined behavior in tor_vasprintf()```
Overflowing a signed integer in C is an undefined behaviour.
It is possible to trigger this undefined behaviour in tor_asprintf on
Windows or systems lacking vasprintf.
On these systems, eiter _vscprintf or vsnprintf is called to re...```
Overflowing a signed integer in C is an undefined behaviour.
It is possible to trigger this undefined behaviour in tor_asprintf on
Windows or systems lacking vasprintf.
On these systems, eiter _vscprintf or vsnprintf is called to retrieve
the required amount of bytes to hold the string. These functions can
return INT_MAX. The easiest way to recreate this is the use of a
specially crafted configuration file, e.g. containing the line:
FirewallPorts AAAAA<in total 2147483610 As>
This line triggers the needed tor_asprintf call which eventually
leads to an INT_MAX return value from _vscprintf or vsnprintf.
The needed byte for \0 is added to the result, triggering the
overflow and therefore the undefined behaviour.
Casting the value to size_t before addition fixes the behaviour.
```Tor: 0.4.0.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30834GetTor depends on Twisted, which has a URL sanitisation vulnerability2020-06-21T18:05:50ZteorGetTor depends on Twisted, which has a URL sanitisation vulnerabilityI'm not sure if GetTor is affected, because the vulnerability depends on user input being put in URLs:
https://github.com/torproject/gettor/network/alert/requirements.txt/twisted/open
Here is a pull request created by GitHub's automated...I'm not sure if GetTor is affected, because the vulnerability depends on user input being put in URLs:
https://github.com/torproject/gettor/network/alert/requirements.txt/twisted/open
Here is a pull request created by GitHub's automated bot:
https://github.com/torproject/gettor/pull/1/fileshttps://gitlab.torproject.org/legacy/trac/-/issues/30109Document that MapAddress is automatically strict, but does not handle redirects2020-06-13T15:40:32ZteorDocument that MapAddress is automatically strict, but does not handle redirectsWe should document that:
1. StrictNodes does not apply to MapAddress
2. MapAddress ~~falls back to a random exit by default~~ fails rather than falling back to a random exit
Edited to add:
3. If the site does a redirect, MapAddress does...We should document that:
1. StrictNodes does not apply to MapAddress
2. MapAddress ~~falls back to a random exit by default~~ fails rather than falling back to a random exit
Edited to add:
3. If the site does a redirect, MapAddress does not apply to the new siteTor: 0.4.0.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/30041OOB access with huge buffers (src/lib/buf/buffers.c)2020-06-13T15:40:19ZGeorge KadianakisOOB access with huge buffers (src/lib/buf/buffers.c)Here is an out-of-bounds read bug found by paldium in hackerone. It's a low-severity bug because it can only be used for DoS, and requires transfer of more than INT_MAX bytes through a connection.
We should backport to 029 and forward a...Here is an out-of-bounds read bug found by paldium in hackerone. It's a low-severity bug because it can only be used for DoS, and requires transfer of more than INT_MAX bytes through a connection.
We should backport to 029 and forward anyhow.
```
# Summary
It is possible to trigger out of boundary accesses with buffers if their content exceeds INT_MAX bytes. See my first patch (0001) how to trigger the issue through unit tests, as this is the easiest way to see what happens. It will result in an out of boundary read. A 64 bit system with at least 5 GB is required for this unit test though!
# Explanation
A buffer consists of one or multiple chunks, which actually contain the data. A chunk contains a memory region and a data pointer. The data pointer points somewhere into the memory, where the actual user data is stored.
Even though a chunk is free to be larger than INT_MAX, it cannot be advised to use such chunks. Whenever a function performs searches or lookups, it is bound to "int" due to buf_pos_t. Many functions properly check that INT_MAX is not exceeded and throw assertions, but unfortunately not all...
Keeping that in mind, I was able to perform a sequence of actions that in fact create chunks with a data length greater than INT_MAX. The int variable "pos" will eventually overflow and access memory far ahead from reserved user data, effectively performing an out of boundary access.
# Exploitation
Generally this is a defensive coding measure to make sure that buffers are safe.
It should be possible to trigger a huge buffer in src/core/mainloop/connection.c. In function connection_buf_read_from_socket a linked connection (directory authentication, as far as I can tell) is joined into the connection buffer with buf_move_to_buf.
The return value of buf_move_to_buf is not properly checked, which means that excessively large data returned from the linked connection can slowly increase the value of "max_to_read" which means that more and more data can be stored in the connection.
Should it eventually overflow INT_MAX (granted, this takes a LOOONG time), the integer calculation will corrupt the buffer, leading to out of boundary operations.
# Patch
To prevent this issue, both patches (0002 to fix buffers and 0003 to check for error return value) must be applied.
## Impact
Heap data is corrupted and out of boundary read access occur. It will be very hard to extract data with that, because it would be a blind memory check -- and will most likely directly cause a segmentation fault.
Most likely attack vector is therefore denial of service.
```Tor: 0.2.9.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/29018Make all statistics depend on ExtraInfoStatistics2020-06-13T15:42:53ZteorMake all statistics depend on ExtraInfoStatisticsLike #29017, when a user sets ExtraInfoStatistics 0, they probably don't want any statistics in their extra-info document.Like #29017, when a user sets ExtraInfoStatistics 0, they probably don't want any statistics in their extra-info document.Tor: 0.4.1.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/28525Make tor_addr_is_internal_() aware of RFC 6598 (Carrier Grade NAT/Large Scale...2020-06-13T15:34:24ZNeel Chauhanneel@neelc.orgMake tor_addr_is_internal_() aware of RFC 6598 (Carrier Grade NAT/Large Scale NAT) IPv4 RangesWith the IPv4 depletion, many ISPs, cell carriers and cloud providers are deploying Carrier Grade NAT with the subnet defined in RFC 6598 (100.64.0.0/10). We should make Tor aware of this range as it is internal as well.
I will write a ...With the IPv4 depletion, many ISPs, cell carriers and cloud providers are deploying Carrier Grade NAT with the subnet defined in RFC 6598 (100.64.0.0/10). We should make Tor aware of this range as it is internal as well.
I will write a patch and give a link to a GitHub PR.Tor: 0.2.9.x-finalNeel Chauhanneel@neelc.orgNeel Chauhanneel@neelc.orghttps://gitlab.torproject.org/legacy/trac/-/issues/27197rust protover accepts excess commas in version strings2020-06-13T15:29:37ZTracrust protover accepts excess commas in version strings
**Trac**:
**Username**: cyberpunks
**Trac**:
**Username**: cyberpunksTor: 0.3.4.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/27194Reject protover extra commas in protover2020-06-13T15:29:36ZTracReject protover extra commas in protoverLike how it handles spaces, `protover.c` rejects leading commas (`Link=,1-5` or `Link=,`) while it accepts and ignores extra commas elsewhere (`Link=1-5,` and `Link=1,,,2-5` are valid).
The Rust version accepts and ignores all extra com...Like how it handles spaces, `protover.c` rejects leading commas (`Link=,1-5` or `Link=,`) while it accepts and ignores extra commas elsewhere (`Link=1-5,` and `Link=1,,,2-5` are valid).
The Rust version accepts and ignores all extra commas, including leading commas.
**Trac**:
**Username**: cyberpunksTor: 0.4.4.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/26522strncat() without bounds2020-06-13T15:27:13ZTracstrncat() without boundsHi Team,
https://github.com/torproject/tor/blob/master/src/lib/err/backtrace.c#L267-L268
i.e
strncat(version, " ", sizeof(version)-1);
strncat(version, tor_version, sizeof(version)-1);
Easily used incorrectly (e.g., incorrectly com...Hi Team,
https://github.com/torproject/tor/blob/master/src/lib/err/backtrace.c#L267-L268
i.e
strncat(version, " ", sizeof(version)-1);
strncat(version, tor_version, sizeof(version)-1);
Easily used incorrectly (e.g., incorrectly computing the correct maximum size to add) such as (CWE-120).
Consider strcat_s, strlcat, snprintf, or automatically resizing strings. I feel the risk is high because the length parameter appears to be a constant, instead of computing the number of characters left.
Request team to please have a look and validate.
Regards
Dhiraj
**Trac**:
**Username**: DhirajTor: 0.3.5.x-finalrl1987rl1987https://gitlab.torproject.org/legacy/trac/-/issues/26488Stop resolving hard-coded authority DNS names that start with a digit2020-06-13T15:27:07ZteorStop resolving hard-coded authority DNS names that start with a digitInstead, we should:
* require IP addresses for authorities in the public network
* allow any DNS name for non-default authorities or in test networks
See #25935 and https://github.com/torproject/tor/pull/59 for background.Instead, we should:
* require IP addresses for authorities in the public network
* allow any DNS name for non-default authorities or in test networks
See #25935 and https://github.com/torproject/tor/pull/59 for background.Tor: 0.3.5.x-finalrl1987rl1987https://gitlab.torproject.org/legacy/trac/-/issues/26301dir-spec: update descriptor on bandwidth changes only when uptime is less tha...2020-06-13T15:26:24Zjugadir-spec: update descriptor on bandwidth changes only when uptime is less than a dayAs commented in https://trac.torproject.org/projects/tor/ticket/24104#comment:19, after changes to the code, the spec should be updatedAs commented in https://trac.torproject.org/projects/tor/ticket/24104#comment:19, after changes to the code, the spec should be updatedTor: 0.3.5.x-finaljugajugahttps://gitlab.torproject.org/legacy/trac/-/issues/26158A little bug of circular path of Tor2020-06-13T15:25:51ZTracA little bug of circular path of TorIn order to defend the **circular-path** attacks, Tor relays detects the next hop and previous hop of a circuit through node-id and Ed25519-id.
However, when the Tor relay detects the previous node has the same Ed25519-id with next nod...In order to defend the **circular-path** attacks, Tor relays detects the next hop and previous hop of a circuit through node-id and Ed25519-id.
However, when the Tor relay detects the previous node has the same Ed25519-id with next node, it forgot to return -1, and continue to extend the circuit.
This might cause some loopholes for the circular-path.
```
/* Next, check if we're being asked to connect to the hop that the
* extend cell came from. There isn't any reason for that, and it can
* assist circular-path attacks. */
if (tor_memeq(ec.node_id,
TO_OR_CIRCUIT(circ)->p_chan->identity_digest,
DIGEST_LEN)) {
log_fn(LOG_PROTOCO[[Image()]]L_WARN, LD_PROTOCOL,
"Client asked me to extend back to the previous hop.");
return -1;
}
/* Check the previous hop Ed25519 ID too */
if (! ed25519_public_key_is_zero(&ec.ed_pubkey) &&
ed25519_pubkey_eq(&ec.ed_pubkey,
&TO_OR_CIRCUIT(circ)->p_chan->ed25519_identity)) {
log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
"Client asked me to extend back to the previous hop "
"(by Ed25519 ID).");
}
```
**Trac**:
**Username**: TBD.ChenTor: 0.3.4.x-final