Jérémy Bobbio authored 13 years ago and Nick Mathewson committed 13 years ago

Original message from bug3393:

check_private_dir() to ensure that ControlSocketsGroupWritable is
safe to use. Unfortunately, check_private_dir() only checks against
the currently running user… which can be root until privileges are
dropped to the user and group configured by the User config option.

The attached patch fixes the issue by adding a new effective_user
argument to check_private_dir() and updating the callers. It might
not be the best way to fix the issue, but it did in my tests.

(Code by lunar; changelog by nickm)

intrigeri authored 13 years ago and Nick Mathewson committed 13 years ago

Fix for bug 3369.

If rep_hist_buffer_stats_write() was called unitinitalized, we'd leak
memory.

This was harmless, we never compared it to anything but itself or 0.
But Coverity complained, and it had a point.

Coverity warned about it, it's harmless to comment out.

George Kadianakis notes that if you give crypto_rand_int() a value
above INT_MAX, it can return a negative number, which is not what
the documentation would imply.

The simple solution is to assert that the input is in [1,INT_MAX+1].
If in the future we need a random-value function that can return
values up to UINT_MAX, we can add one.

Fixes bug 3306; bugfix on 0.2.2pre14.

When we added the check for key size, we required that the keys be
128 bytes.  But RSA_size (which defers to BN_num_bytes) will return
128 for keys of length 1017..1024.  This patch adds a new
crypto_pk_num_bits() that returns the actual number of significant
bits in the modulus, and uses that to enforce key sizes.

Also, credit the original bug3318 in the changes file.

UseBridges 1 now means "connect only to bridges; if you know no
bridges, don't make connections."  UseBridges auto means "Use bridges
if they are known, and we have no EntryNodes set, and we aren't a
server."  UseBridges 0 means "don't use bridges."

options->DirPort is 0 in the unit tests, so
router_get_advertised_dir_port() would return 0 so we wouldn't pick a
dirport. This isn't what we want for the unit tests. Fixes bug
introduced in 95ac3ea5.

I hope these will never be useful, but having them and not needing them is
better than needing them and not having them.

Fixes bug #3309.

Previously, Tor would dereference a NULL pointer and crash if
lookup_last_hid_serv_request were called before the first call to
directory_clean_last_hid_serv_requests.  As far as I can tell, that's
currently impossible, but I want that undocumented invariant to go away
in case I^Wwe break it someday.

Gisle authored 13 years ago and Nick Mathewson committed 13 years ago

An elusive compile-error (MingW-gcc v4.50 on Win_XP); a missing
comma (!) and a typo ('err_msg' at line 277 changed to 'errmsg').
Aso changed the format for 'err_code' at line 293 into a "%ld" to suppress
a warning. How did this go unnoticed for ~1 month? Btw. This is my 1st ever
'git commit', so it better work.

When we introduced NEED_KEY_1024 in routerparse.c back in
0.2.0.1-alpha, I forgot to add a *8 when logging the length of a
bad-length key.

Bugfix for 3318 on 0.2.0.1-alpha.

If you had configured a bridge but then switched to a different bridge
via the controller, you would still be willing to use the old one.

The patch for 3228 made us try to run init_keys() before we had loaded
our state file, resulting in an assert inside init_keys. We had moved
it too early in the function.

Now it's later in the function, but still above the accounting calls.

Sebastian Hahn authored 13 years ago and Nick Mathewson committed 13 years ago

This simple implementation has a few issues, but it should do for
0.2.2.x. We will want to revisit this later and make it smarter.

Conflicts:
	src/or/circuitbuild.c

The comment fixes are trivial.  The defensive programming trick is to
tolerate receiving NULL inputs on the describe functions. That should
never actually happen, but it seems like the likeliest mistake for us
to make in the future.