Skip to content

fix(charts): fixed out of bounds read#6607

Merged
ehsandeep merged 1 commit intoprojectdiscovery:devfrom
Deamhan:fix_charts_panic
Nov 14, 2025
Merged

fix(charts): fixed out of bounds read#6607
ehsandeep merged 1 commit intoprojectdiscovery:devfrom
Deamhan:fix_charts_panic

Conversation

@Deamhan
Copy link
Contributor

@Deamhan Deamhan commented Nov 12, 2025

Fixed panic during statistics report generation in case of small amount of templates loaded. Before fix the following issue takes place:

[+] Scan Info

Name: nuclei-stats
Target Count: 20
Template Count: 35
Template Concurrency: 25
Payload Concurrency: 25
Retries: 1
Total Events: 1400
panic: runtime error: slice bounds out of range [:50] with capacity 35

goroutine 1 [running]:
github.com/projectdiscovery/nuclei/v3/pkg/scan/charts.(*ScanEventsCharts).topSlowTemplates(0xc000504de0, {0x1?, 0x0?})
github.com/projectdiscovery/nuclei/v3/pkg/scan/charts/echarts.go:177 +0x14f1
github.com/projectdiscovery/nuclei/v3/pkg/scan/charts.(*ScanEventsCharts).allCharts(0xc000504de0, {0x0, 0x0})
github.com/projectdiscovery/nuclei/v3/pkg/scan/charts/echarts.go:45 +0xa5
github.com/projectdiscovery/nuclei/v3/pkg/scan/charts.(*ScanEventsCharts).GenerateHTML(0xc000504de0?, {0x7ffef77c9ffa, 0x26})
github.com/projectdiscovery/nuclei/v3/pkg/scan/charts/echarts.go:28 +0x37
main.main()
./main.go:33 +0x1f4

To reproduce the issue 2 steps are required:

  1. run nuclei with less than 50 templates loaded and statistics enabled (build-stats)
  2. generate an html report using scan-charts

Summary by CodeRabbit

  • Bug Fixes
    • Fixed handling of chart display when fewer data points are available than the display limit. The chart now gracefully displays the actual number of items available.
    • Updated chart titles to accurately reflect the actual count of data points displayed.

@auto-assign auto-assign bot requested a review from dogancanbakir November 12, 2025 20:04
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025

Walkthrough

Modified the topSlowTemplates function in echarts.go to prevent out-of-bounds access by dynamically limiting iteration to the minimum of TopK and available data length, with the chart title updated to reflect the actual element count.

Changes

Cohort / File(s) Summary
Boundary condition fix
pkg/scan/charts/echarts.go
Computes dynamic limit (min(TopK, len(data))) to guard against accessing more elements than available; updates chart title to reflect actual data points instead of always using TopK

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single-file change with straightforward boundary logic
  • Review focus: Verify the min() calculation correctness and confirm chart title accurately reflects available data count

Poem

🐰 A boundary we now guard with care,
No more reaching beyond what's there!
Dynamic limits, safe and sound,
Where data truly can be found. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main change: fixing an out-of-bounds read bug in the charts module that was causing panics.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cf5557e and 076deec.

📒 Files selected for processing (1)
  • pkg/scan/charts/echarts.go (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Format Go code using go fmt
Run static analysis with go vet

Files:

  • pkg/scan/charts/echarts.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Lint
🔇 Additional comments (3)
pkg/scan/charts/echarts.go (3)

175-179: Excellent fix for the out-of-bounds panic!

The dynamic limit calculation correctly prevents the slice bounds panic when fewer than TopK templates are available. This directly addresses the issue described in the PR objectives.


183-183: Correct application of the dynamic limit.

The slice operation now safely uses the calculated limit, ensuring we never exceed the available data length while still selecting the top slowest templates after sorting.


190-190: Ignore this review comment — the concern is based on incorrect library behavior assumptions.

The review comment assumes that SetGlobalOptions preserves the subtitle from the first call, but this is incorrect. According to go-echarts documentation, SetGlobalOptions overwrites/sets the chart's global option fields rather than performing an incremental deep merge — most GlobalOpts provided to SetGlobalOptions replace the previous value for that option.

When the second SetGlobalOptions call executes at line 189, it replaces the entire global options from the first call (line 133–138). The subtitle set at line 136 is overwritten, not preserved. Therefore, there is no mismatch between title and subtitle numbers — the subtitle is simply not present after line 189.

The code is correct as-is. No changes are needed.

Likely an incorrect or invalid review comment.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ehsandeep ehsandeep merged commit 488d588 into projectdiscovery:dev Nov 14, 2025
20 checks passed
@ehsandeep ehsandeep removed the request for review from dogancanbakir November 14, 2025 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants