-
-
Notifications
You must be signed in to change notification settings - Fork 676
feat: Support for adding taint analysis engine #1486
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
base: master
Are you sure you want to change the base?
feat: Support for adding taint analysis engine #1486
Conversation
242671d to
105052f
Compare
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1486 +/- ##
==========================================
+ Coverage 68.49% 69.36% +0.86%
==========================================
Files 75 88 +13
Lines 4384 7089 +2705
==========================================
+ Hits 3003 4917 +1914
- Misses 1233 1924 +691
- Partials 148 248 +100 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@ccojocar Please review when you get a chance. TIA |
a653518 to
a5dd1eb
Compare
Implements SSA-based taint analysis to detect security vulnerabilities: - G701: SQL injection via string concatenation - G702: Command injection via user input - G703: Path traversal via user input - G704: SSRF via user-controlled URLs - G705: XSS via unescaped user input - G706: Log injection via user input Uses golang.org/x/tools for SSA/call graph analysis with CHA. Zero external dependencies beyond existing gosec imports.
a5dd1eb to
bf684e2
Compare
|
Thanks for submitting this. I will need some time to review it. Which AI code generator are you using? |
Thank you for asking @ccojocar. Claude helped with initial scaffolding, and I handled the refinements and final implementation to improve further. Please take your time reviewing. I am interested to see this reach a wider audience and encourage broader gosec adoption. Thank you for your time again. |
|
|
||
| // Source defines where tainted data originates. | ||
| // Format: "package/path.TypeOrFunc" or "*package/path.Type" for pointer types. | ||
| type Source struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a go doc comment on each field?
| // Format: "package/path.TypeOrFunc" or "*package/path.Type" for pointer types. | ||
| type Source struct { | ||
| Package string // e.g., "net/http" | ||
| Name string // e.g., "Request" or "Get" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this function/method name? Can you make the name more clear?
|
|
||
| // Sink defines a dangerous function that should not receive tainted data. | ||
| // Format: "(*package/path.Type).Method" or "package/path.Func" | ||
| type Sink struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add go doc on each field.
| } | ||
|
|
||
| // Result represents a detected taint flow from source to sink. | ||
| type Result struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a go doc on each field.
|
|
||
| // Analyzer performs taint analysis on SSA programs. | ||
| type Analyzer struct { | ||
| config Config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use a pointer for config.
| {Package: "net/http", Method: "Get"}, | ||
| {Package: "net/http", Method: "Post"}, | ||
| {Package: "net/http", Method: "Head"}, | ||
| {Package: "net/http", Method: "PostForm"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
http.NewRequest missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
net.Dial, net.DialTImeout, net.LookupHost
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
net/http/httputil.ReverseProxy:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
net/http/httputil NewSingleHostReverseProxy
| // XSS returns a configuration for detecting Cross-Site Scripting vulnerabilities. | ||
| func XSS() Config { | ||
| return Config{ | ||
| Sources: []Source{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see sources above for a more complete list
| {Package: "net/http", Name: "Request", Pointer: true}, | ||
| {Package: "net/url", Name: "Values"}, | ||
| }, | ||
| Sinks: []Sink{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
template.HTML, template.HTMLAttr, template.JS, template.CSS missing
| func LogInjection() Config { | ||
| return Config{ | ||
| Sources: []Source{ | ||
| {Package: "net/http", Name: "Request", Pointer: true}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above for a more complete list of sources
| {Package: "log", Method: "Println"}, | ||
| {Package: "log", Method: "Fatal"}, | ||
| {Package: "log", Method: "Fatalf"}, | ||
| {Package: "log", Method: "Fatalln"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log.Panic, log/slog.Info, Error, Warn
Taint Analysis Engine for gosec
Implements a minimal, zero-dependency taint analysis engine for gosec that tracks data flow from sources (user input) to sinks (dangerous functions) to detect security vulnerabilities.
Issue: #1160 - Request for taint analysis support in gosec
New Security Rules
Changes
golang.org/x/toolspackages that gosec already depends onExample Detection
SQL Injection (G701):
Command Injection (G702):