wbenny / mini-tor

proof-of-concept implementation of tor protocol using Microsoft CNG/CryptoAPI
MIT License
393 stars 89 forks source link

Bugs: Does _recv_cell_loop_thread join itself? Is abnormal tor_stream not freed? #12

Open padicao2010 opened 6 years ago

padicao2010 commented 6 years ago
  1. _recv_cell_loop_thread is started to run function _recv_cell_loop. But when a received cell is invalid. _recv_cell_loop calls close(). Because the state cannot be closing, so it would call _recv_cell_loop_thread.join and _recv_cell_loop_thread.reset.

    Is it a bug? Maybe line 447 of tor_socket.cpp should be removed?

  2. At circuit::create_stream, when tor_stream is not ready, it is destroyed otherwhere, but it is not deleted. Would it be a memory leak?

  3. At circuit::send_relay_end_cell, it set_state to destroyed and removes the stream from _stream_map. At circuit::handle_relay_end_cell, it does these again. set_state seems thread-safe, but _stream_map seems not. Would it cause a problem?

wbenny commented 6 years ago

Hi, I'm sorry, but I have quite hard time to understand some of your points.

  1. Why it would be a bug? Also, tor_socket.cpp:447 shows closing parenthesis of tor_socket::recv_certificates function. Did you mean another line?

  2. Valid, thank you. If error occurs while creating stream, the memory for the tor_stream is not freed.

  3. I need better explanation of this. Why would it cause problem? send_relay_end_cell is used when client (you) wants to close the stream, whereas handle_relay_end_cell is called when server (tor node) wants to close the stream.

padicao2010 commented 6 years ago
  1. Sorry, I made a mistake. It is not a problem. But sometimes it throws exception at _recv_cell_loop_thread.join because the thread is null. I don't know why and I just add a check before join.

  2. Nothing.

  3. Sorry, I made a mistake, too. It is not the problem. But sometimes at close_streams/send_relay_end_cell , an exception would arise. At watching window, _stream_map.is_empty() returns false, but _stream_map.get_size() returns 0, _stream_map.last_value() returns an invalid pointer.