readWithTimeout uses incorrect logic

[MegaManSec]

Per-call readWithTimeout uses full timeout each iteration causing cumulative blocking in HTTPConnector::recv_message (DoS) (L351-L375) (medium) (Confidence: 8.5) (Severity: 9.2)

The function readWithTimeout defined in pdns/sstuff.hh (lines 357–369) calls waitForRWData(d_socket, true, timeout, 0) and therefore uses the supplied timeout as a per-call blocking wait. In modules/remotebackend/httpconnector.cc,
HTTPConnector::recv_message (lines 437–446) enforces an overall timeout by tracking time0 and looping, but inside that loop it repeatedly invokes d_socket->readWithTimeout(..., timeout) passing the original timeout each iteration. Because each call can block up to the full timeout, multiple iterations can cumulatively exceed the intended overall timeout, resulting in threads blocking for far longer than expected and risking resource exhaustion or denial of service.

This bug was found with ZeroPath.

[omoerbeek]

The loop has a guard (labs(time(nullptr) - time0) <= timeout, so I don't think the scneraio you sketch can happen.
The first iteration might use timeout seconds, but then the second iteration certainly will certainly make the guard expression fail. So the max total timeout that can happen is 2*timeout, and not a unbounded value as you suggest.

[MegaManSec]

Indeed, just double checked and yes, it's just 2x, I agree

[omoerbeek]

Also: in a way a remote backend should considered a trusted entity.

[MegaManSec]

I agree; this is just a minor implementation bug