-
Notifications
You must be signed in to change notification settings - Fork 409
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
neon extension: add neon_pgdump_schema function #7492
Conversation
it can be used to execute pg_dump utility in the containerized environment of compute
errmsg("Failed to build command"))); | ||
} | ||
|
||
pipe = popen(command, "r"); |
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.
Static Code Analysis Risk: CWE 78 - Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') - Command injection
Command injection vulnerability in C/C++ allows attackers to execute arbitrary OS commands by manipulating externally-influenced input due to improper sanitization. Avoid using external input in OS commands. If unavoidable, strictly validate and sanitize the input against a predefined whitelist before execution.
Severity: High 🚨
Status: Open 🔴
References:
- https://cwe.mitre.org/data/definitions/676
- https://cwe.mitre.org/data/definitions/78
- https://cwe.mitre.org/data/definitions/88
- https://www.sei.cmu.edu/downloads/sei-cert-c-coding-standard-2016-v01.pdf
You received this notification because a new code risk has been identified
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.
@prepor this code snippet looks vulnerable to OS injection, did you have a chance to look at this alert?
I'd suggest putting this in |
text *ret = cstring_to_text(buf.data); | ||
pfree(buf.data); | ||
PG_RETURN_TEXT_P(ret); |
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.
There's a 1 GB limit on the size of one datum in Postgres, so this will fail if the output was larger than 1 GB. That'd a large schema, but not impossible.
|
||
const char *port = GetConfigOptionByName("port", NULL, false); | ||
|
||
int command_res = snprintf(command, sizeof(command), "pg_dump --schema-only -h %s -p %s -U %s -d %s", COMPUTEHOST, port, COMPUTEROLE, dbname); |
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.
'dbname' needs escaping / quoting
2766 tests run: 2627 passed, 18 failed, 121 skipped (full report)Failures on Postgres 16
Failures on Postgres 15
Failures on Postgres 14
Test coverage report is not availableThe comment gets automatically updated with the latest test results
87c53cd at 2024-04-24T09:19:57.768Z :recycle: |
We decided to go with a different approach, because we can't rely on extension functions during read-only time travel |
Adds neon_pgdump_schema function
It can be used to execute pg_dump utility in the containerized environment of compute