Fix memory leak in tcp server by wiping priority queues on deletion.

[zugz]

This change is 

[iphydf]

It'll need to be called something else. https://travis-ci.org/TokTok/c-toxcore/jobs/434939506#L1504

[zugz]

It turns out that this closes #1214, as this was the memleak exploited there.
@kpp found the flaw first, and chose to communicate it to us non-verbally in this way.
@kpp and I just tested that this patch does nullify that attack.

[robinlinden]

Reviewed 3 of 3 files at r2.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @iphydf)

[kpp]

https://github.com/tox-rs/tox is your best friend =) The second 0day this year.

[zugz]

This doesn't actually fix all the potential memleaks yet. e.g. in kill_TCP_server the TCP_Secure_Connections aren't wiped, so there could be unfreed priority queues. Similarly in realloc_connection. I'm also worried about the potential for double frees. This is going to take some care... is there anyone who's actually familiar with TCP_server.c and wouldn't mind helping out?

[nurupo]

@zugz what do you mean by "non-verbally"? How exactly did he communicate? Was it by taking down 3/4 of bootstrap nodes? Because if so, then that's a rather malicious way of communicating and tox-rs ain't no friend.

[zugz]

* Saturday, 2018-09-29 at 12:36 -0700 - nurupo <notifications@github.com>:

@zugz what do you mean by "non-verbally"? How exactly did he communicate? Was it by taking down 3/4 of bootstrap nodes?

That is what I meant.

[codecov]

Codecov Report

Merging #1216 into master will decrease coverage by 0.1%.
The diff coverage is 85%.

@@           Coverage Diff            @@
##           master   #1216     +/-   ##
========================================
- Coverage    82.8%   82.7%   -0.2%     
========================================
  Files          82      82             
  Lines       14634   14647     +13     
========================================
- Hits        12125   12120      -5     
- Misses       2509    2527     +18

Impacted Files	Coverage Δ
toxcore/TCP_client.c	64.6% <100%> (+0.3%)	⬆️
toxcore/TCP_server.c	75.9% <84.6%> (+0.9%)	⬆️
auto_tests/toxav_many_test.c	92.5% <0%> (-4.1%)	⬇️
toxcore/net_crypto.c	93.5% <0%> (-1.4%)	⬇️
toxav/msi.c	63.6% <0%> (-1.2%)	⬇️
toxav/toxav.c	67.1% <0%> (-1%)	⬇️
toxcore/Messenger.c	86.6% <0%> (+0.1%)	⬆️
toxcore/friend_connection.c	94.5% <0%> (+0.8%)	⬆️
auto_tests/toxav_basic_test.c	83.5% <0%> (+1.9%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2b96b1...5beb00c. Read the comment docs.

[zugz]

OK, this is ready for (re)review now, and I believe it now fully seals this source of memleaks rather than just dealing with the case exploited in #1214. There wasn't nearly as much further work as I was fearing in my previous comment.

A word on my methodology here: I haven't actually grokked the entirety of TCP_server.c; rather, I searched for "TCP_Secure_Connection" and followed up on each hit. I don't see that I could be missing anything. Also, with these latest changes asan now passes on #1215 - and asan's previous failure there was what directly led me to finding the memleak, which incidentally I think we can read as a win for our testing regime!

Anyway, please review carefully.

[robinlinden]

Reviewed 1 of 1 files at r4.
Reviewable status: complete! 1 change requests, 1 of 1 approvals obtained (waiting on @iphydf)

---

toxcore/TCP_server.c, line 134 at r4 (raw file):

static int alloc_new_connections(TCP_Server *tcp_server, uint32_t num)
{
    uint32_t new_size = tcp_server->size_accepted_connections + num;

const

[irungentoo]

This is how I fixed it: irungentoo@bf69b54

[sudden6]

from my side

Reviewed 2 of 3 files at r2, 1 of 1 files at r5.
Reviewable status: complete! 1 change requests, 1 of 1 approvals obtained (waiting on @iphydf)