projectdiscovery / naabu

A fast port scanner written in go with a focus on reliability and simplicity. Designed to be used in combination with other tools for attack surface discovery in bug bounties and pentests
https://projectdiscovery.io
MIT License
4.8k stars 553 forks source link

Goroutine Leak ConnectVerfication() -> Limitter #1250

Closed awerqo closed 1 month ago

awerqo commented 1 month ago

When invoke ConnectVerfication() func in runner it creates new limiter on line 717 https://github.com/projectdiscovery/naabu/blob/10f56057677681f41e4d13a4929820c071d7215f/v2/pkg/runner/runner.go#L714-L717

Creating new instances of Limiter starts new goroutine: https://github.com/projectdiscovery/ratelimit/blob/77dad731f2e9a2e98564787086ade7be2ac33b4e/ratelimit.go#L112-L129

And it closed only if any of two contexts done https://github.com/projectdiscovery/ratelimit/blob/main/ratelimit.go#L39-L45

select {
case <-ctx.Done():
    // Internal Context
    imiter.ticker.Stop()
    eturn
case <-limiter.ctx.Done():
    limiter.ticker.Stop()
    return
case ...

So this happens if original context done or if we call stop method https://github.com/projectdiscovery/ratelimit/blob/77dad731f2e9a2e98564787086ade7be2ac33b4e/ratelimit.go#L101-L106 that cancel second, created context https://github.com/projectdiscovery/ratelimit/blob/77dad731f2e9a2e98564787086ade7be2ac33b4e/ratelimit.go#L113

As we can see, when we invoke ConnectVerification() it creates new instance of limiter with context.Background() and not invoke Stop() later: https://github.com/projectdiscovery/naabu/blob/10f56057677681f41e4d13a4929820c071d7215f/v2/pkg/runner/runner.go#L717-L733

So, we leak goroutine every ConnectVerification() call, what sometimes creates problems if we work with naabu in SDK mode

Thanks

awerqo commented 1 month ago

I can send a pr if it is convenient for you. It looks like it is safe to call limiter.Stop() after waiting for the swg