[tor-bugs] #33889 [Core Tor/Tor]: Functions with char* arguments are dangerous when used with casting

Tor Bug Tracker & Wiki blackhole at torproject.org
Mon Apr 13 11:16:51 UTC 2020


#33889: Functions with char* arguments are dangerous when used with casting
------------------------------------+------------------------------------
 Reporter:  asn                     |          Owner:  (none)
     Type:  defect                  |         Status:  new
 Priority:  Medium                  |      Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor            |        Version:
 Severity:  Normal                  |     Resolution:
 Keywords:  security memory-safety  |  Actual Points:
Parent ID:                          |         Points:  3
 Reviewer:                          |        Sponsor:
------------------------------------+------------------------------------
Description changed by asn:

Old description:

> There are various functions in our codebase which receive char* arguments
> when they actually receive a byte array. That's I guess because of the
> old C standard when uint8_t didn't really exist, so we had to use char*
> for everything.
>
> Still, these days we use uint8_t* and this means that we need to cast it
> to char* when we use those functions. This can cause security issues
> because this casting is silencing type-safety warnings (as an example see
> https://github.com/torproject/tor/pull/1843#discussion_r406191281).
>
> For example, see how `fast_mem_is_zero()` is used 75% of the time with
> casting its first argument. Instead of doing this, we could make a new
> `fast_mem_is_zero_uint8t()` function that takes uint8_t as argument. Or
> even make a `fast_mem_is_zero_char()` function that takes char*.
>
> In other functions, it might be possible to replace the `char*` with
> `void*` but in the case of `fast_mem_is_zero()` that's not possible
> because of the implementation of the function.
>
> Other potential problem functions: `base64_encode()` `hex_str()`
> `crypto_rand()` `crypto_digest()`.

New description:

 There are various functions in our codebase which receive char* arguments
 when they actually receive a byte array. That's I guess because of the old
 C standard when uint8_t didn't really exist, so we had to use char* for
 everything.

 Still, these days we use uint8_t* and this means that we need to cast it
 to char* when we use those functions. This can cause security issues
 because this casting is silencing type-safety warnings (as an example see
 https://github.com/torproject/tor/pull/1843#discussion_r406191281 from
 #33545).

 For example, see how `fast_mem_is_zero()` is used 75% of the time with
 casting its first argument. Instead of doing this, we could make a new
 `fast_mem_is_zero_uint8t()` function that takes uint8_t as argument. Or
 even make a `fast_mem_is_zero_char()` function that takes char*.

 In other functions, it might be possible to replace the `char*` with
 `void*` but in the case of `fast_mem_is_zero()` that's not possible
 because of the implementation of the function.

 Other potential problem functions: `base64_encode()` `hex_str()`
 `crypto_rand()` `crypto_digest()`.

--

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/33889#comment:1>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online


More information about the tor-bugs mailing list