-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[INS-243] Fix jdbc detector detecting incomplete connection string and fixed invalid… #4636
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
[INS-243] Fix jdbc detector detecting incomplete connection string and fixed invalid… #4636
Conversation
… parsing of sql server string
nabeelalam
left a comment
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.
Looks good @MuneebUllahKhan222. Could you update this branch?
2a1bc2f to
86c8d22
Compare
* [INS-165] Fix Jdbc tests and updated the deprecated code * Remove commented-out typo from sqlserver test
86c8d22 to
daa94b1
Compare
f13722a to
30b4660
Compare
camgunz
left a comment
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.
Just a couple non-blocking Qs; feel free to merge 👍🏻
| var ( | ||
| keyPat = regexp.MustCompile(`(?i)jdbc:[\w]{3,10}:[^\s"'<>,(){}[\]&]{10,512}`) | ||
| // Matches typical JDBC connection strings amd ingores any special character at the end | ||
| keyPat = regexp.MustCompile(`(?i)jdbc:[\w]{3,10}:[^\s"'<>,{}[\]]{10,511}[A-Za-z0-9]`) |
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.
question: This changed from "{10,512}" to "{10,511}". Should it be "{9,511}"?
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.
No it should be {10,511} as we don't want to change the minimum number of characters that can occur.
Description:
Earlier, we found that the JDBC detector was missing certain valid connection strings, which led to unverified secrets in scans:
Connection strings with query parameters (e.g., ?user=...&password=...) were not matched by the regex.
JDBC URLs containing parentheses (e.g.,
jdbc:mysql://testuser:testpassword@tcp(localhost:1521)/testdb) were also not matched.The
ParseSqlServermethod failed to correctly extract host and port from strings with a driver prefix like//odbc:server=localhost;port=1433;database=testdb;password=testpassword.This PR fixes these issues by:
Updating the JDBC regex to allow query parameters and parentheses inside the connection string while still preventing trailing special characters.
Enhancing
ParseSqlServerto correctly handle connection strings with optional driver prefixes like odbc: and extract host and port reliably.Checklist:
make test-community)?make lintthis requires golangci-lint)?