fix buffer use after it was released when sending an INVALID_TOKEN error #2524
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2524 +/- ##
==========================================
- Coverage 86.14% 86.14% -0.00%
==========================================
Files 122 122
Lines 9536 9535 -1
==========================================
- Hits 8214 8213 -1
Misses 984 984
Partials 338 338
Continue to review full report at Codecov.
|
marten-seemann
force-pushed
the
fix-buffer-use-after-release
branch
from
8bb5cea
to
aebf238
Compare
| @@ -292,7 +292,7 @@ func (s *baseServer) handlePacket(p *receivedPacket) { | |||
| } | |||
| } | |||
|
|
|||
| func (s *baseServer) handlePacketImpl(p *receivedPacket) bool /* was the packet handled */ { | |||
| func (s *baseServer) handlePacketImpl(p *receivedPacket) bool /* should the buffer be released */ { | |||
There was a problem hiding this comment.
This seems quite brittle, similar to manual mem management in C. Is there a way to have all free-ing always be done by a single function? If not, maybe it's worth copying the buf contents in those cases? I think we could take a small perf hit over the risk of serious security issues…?
Is there a way we can avoid similar bugs in the future, make sure we always release buffers properly? With tests we probably can't guarantee full coverage.
There was a problem hiding this comment.
The race detector will help us with that. In fact, that's how I found this issue.
When you use a buffer that you already released, and this buffer is used again, this is a race condition. Even if the detection is not 100% guaranteed, at least the test will be flaky...
Related to #2520.