Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix multiple mem leaks + optimizations #4630

Merged
merged 9 commits into from
Jan 18, 2024
Merged

fix multiple mem leaks + optimizations #4630

merged 9 commits into from
Jan 18, 2024

Conversation

tarunKoyalwar
Copy link
Member

@tarunKoyalwar tarunKoyalwar commented Jan 12, 2024

Proposed Changes

This PR brings multiple memory optimizations and fixes recently found memory leaks

Bug 1) Missing Bounds check on http response body cause over consumption of memory ( ex: if server returns 500MB response that was loaded / evaluated etc . which caused nuclei to crash)

Bug 2) Unnecessary creation of contextargs when multiprotocol/flow is not used ( i.e for normal templates )

A long term solution for this is #4631 . this will also remove redundant creation of variables everytime we need to evaluate and build request ( expected memory decrease after implementation ~50%)

Bug 3) skip evaluating dynamic variables after creation of request

Probably due to conflicting logic and changes in logic it was found that we were evaluating all variables of a template (i.e templateCtx) and when evaluating dsl runs a FindExpression function to resolve nested expression and here time and space complexity corresponds to length of expression (and since there was already a case of missing bounds check this caused memory leak)

Note

As solution for #bug3 we removed old logic because its not needed any more ( tested locally to see if the map contained any un-evaluated variables i.e {{Path}} etc something like that . but didn't find any.
User reported that this change might have affected some functionality and some CVE results are missing but this hasn't been confimed / reproduced from our side yet

Bug4) Re-creation of fastdialer everytime we reference default client , options or transport from retryablehttp-go

All default options,clients,transports in retyrablehttp reference DefaultReuseableTransport which created new fastdialer instance any time one of these defaults were referenced/called . This lead to un-necessary creation of fastidialer and memory leak due to bug in fastdialer itself

Bug5) loading hostfile everytime new fastdialer is created

Although fastdialer was not designed this way due to requirement in nuclei/retryablehttp etc fastdialer was created multiple times (for xyz config) and everytime fastdialer was created we read all data from /etc/hosts to memory (which caused issue like #4632

Bug6) Missing bounds check on /etc/hosts in fastdialer and

we have already implemented bounds check (i.e max host entries to load) in retryabledns but this logic was missing in fastdialer which again affected total data i.e being loaded in memory (especially if /etc/hosts file is large) . this was also a cause behind #4632

@tarunKoyalwar tarunKoyalwar self-assigned this Jan 12, 2024
This was referenced Jan 12, 2024
@tarunKoyalwar
Copy link
Member Author

POC for #4632

$  wc -l /etc/hosts
     920 /etc/hosts

Nuclei v3.1.5

$ cmdutil nuclei

                     __     _
   ____  __  _______/ /__  (_)
  / __ \/ / / / ___/ / _ \/ /
 / / / / /_/ / /__/ /  __/ /
/_/ /_/\__,_/\___/_/\___/_/   v3.1.5

		projectdiscovery.io

[INF] Current nuclei version: v3.1.5 (latest)
[INF] Current nuclei-templates version: v9.7.3 (latest)
[WRN] Scan results upload to cloud is disabled.
[INF] New templates added in latest release: 46
[INF] Templates loaded for current scan: 7426
[INF] Executing 7448 signed templates from projectdiscovery/nuclei-templates
[INF] No results found. Better luck next time!

------------------------------
Command: nuclei
Max RSS: 5396 MB
Sys Time: 536.258µs
User Time: 558.783µs
Actual Time: 2m3.176838083s
Voluntary Context Switch (nvcsw): 9715

This branch

$ cmdutil ./nuclei

                     __     _
   ____  __  _______/ /__  (_)
  / __ \/ / / / ___/ / _ \/ /
 / / / / /_/ / /__/ /  __/ /
/_/ /_/\__,_/\___/_/\___/_/   v3.1.6-dev

		projectdiscovery.io

[INF] Current nuclei version: v3.1.6-dev (development)
[INF] Current nuclei-templates version: v9.7.3 (latest)
[WRN] Scan results upload to cloud is disabled.
[INF] New templates added in latest release: 46
[INF] Templates loaded for current scan: 7426
[INF] Executing 7448 signed templates from projectdiscovery/nuclei-templates
[INF] No results found. Better luck next time!

------------------------------
Command: ./nuclei
Max RSS: 334 MB
Sys Time: 561.432µs
User Time: 150.796µs
Actual Time: 11.957603583s
Voluntary Context Switch (nvcsw): 8159

@tarunKoyalwar tarunKoyalwar changed the title fix mem leak fix multiple mem leaks + optimizations Jan 16, 2024
@tarunKoyalwar tarunKoyalwar marked this pull request as ready for review January 16, 2024 14:39
@tarunKoyalwar
Copy link
Member Author

Improvements between single target scan in latest release and this PR

Current release i.e v3.1.5

------------------------------
Command: nuclei -u scanme.sh -c 100 -stats
Max RSS: 566 MB
Sys Time: 69.303µs
User Time: 746.993µs
Actual Time: 3m14.976412625s
Voluntary Context Switch (nvcsw): 755

This PR

Command: ./nuclei -u scanme.sh -c 100 -stats
Max RSS: 410 MB
Sys Time: 6.936µs
User Time: 73.32µs
Actual Time: 3m15.477689209s
Voluntary Context Switch (nvcsw): 9651
  • flat difference of 100-150MB in all scans (for 1 or more targets)
  • 10x less cpu usage

@ehsandeep ehsandeep merged commit c7c35ff into dev Jan 18, 2024
9 of 12 checks passed
@ehsandeep ehsandeep deleted the fix-mem-leak branch January 18, 2024 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants