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

CGI environment variables need a prefix #249

Closed
mobrien opened this Issue Jun 9, 2017 · 3 comments

Comments

Projects
None yet
1 participant
@mobrien
Contributor

mobrien commented Jun 9, 2017

CGI variables from request queries and from POST data are passed to CGI programs. These variables correspond to the user parameters.

There needs to be a prefix applied to avoid clashing with standard environment variables.

@mobrien mobrien added this to the 3.6.5 milestone Jun 9, 2017

@mobrien

This comment has been minimized.

Show comment
Hide comment
@mobrien

mobrien Jun 9, 2017

Contributor

This can be used to impact dynamically linked CGI programs by setting LD_ environment variables. I know most CGI programs are statically linked and we generally recommend not using CGI in the first place. But this can be exploited ... more to come.

Contributor

mobrien commented Jun 9, 2017

This can be used to impact dynamically linked CGI programs by setting LD_ environment variables. I know most CGI programs are statically linked and we generally recommend not using CGI in the first place. But this can be exploited ... more to come.

@mobrien

This comment has been minimized.

Show comment
Hide comment
@mobrien

mobrien Jun 9, 2017

Contributor

These details were reported by Daniel Hodson at Elttam: https://www.elttam.com.au/blog/ in some excellent security research.

This issue can result in a remote code execution vulnerability on Linux with CGI dynamically linked programs.

The GoAhead CGI handler accepts HTTP query and form data parameters and creates CGI environment variables for each parameter. On Linux, if a parameter of the name LD_PRELOAD is supplied and set to the standard input, the POST data may be accepted as code and may be pre-loaded into dynamically linked CGI processes before they run. This permits arbitrary code injection into the CGI process on Linux.

To exploit the vulnerability, an attacker would create a HTTP CGI request that sets LD_PRELOAD=/proc/self/fd/0 in the query string and sets the POST data of the request to be in the form of a malicious shared library for the architecture of the device.

Here is an isolated patch that can be applied immediately.

diff --git a/src/cgi.c b/src/cgi.c
index 899ec97..18d9b45 100644
--- a/src/cgi.c
+++ b/src/cgi.c
@@ -160,10 +160,17 @@ PUBLIC bool cgiHandler(Webs *wp)
     envpsize = 64;
     envp = walloc(envpsize * sizeof(char*));
     for (n = 0, s = hashFirst(wp->vars); s != NULL; s = hashNext(wp->vars, s)) {
-        if (s->content.valid && s->content.type == string &&
-            strcmp(s->name.value.string, "REMOTE_HOST") != 0 &&
-            strcmp(s->name.value.string, "HTTP_AUTHORIZATION") != 0) {
-            envp[n++] = sfmt("%s=%s", s->name.value.string, s->content.value.string);
+        if (s->content.valid && s->content.type == string) {
+            if (smatch(s->name.value.string, "REMOTE_HOST") ||
+                smatch(s->name.value.string, "HTTP_AUTHORIZATION") ||
+                smatch(s->name.value.string, "IFS") ||
+                smatch(s->name.value.string, "CDPATH") ||
+                smatch(s->name.value.string, "PATH") ||
+                sstarts(s->name.value.string, "LD_")) {
+                continue;
+            }
Contributor

mobrien commented Jun 9, 2017

These details were reported by Daniel Hodson at Elttam: https://www.elttam.com.au/blog/ in some excellent security research.

This issue can result in a remote code execution vulnerability on Linux with CGI dynamically linked programs.

The GoAhead CGI handler accepts HTTP query and form data parameters and creates CGI environment variables for each parameter. On Linux, if a parameter of the name LD_PRELOAD is supplied and set to the standard input, the POST data may be accepted as code and may be pre-loaded into dynamically linked CGI processes before they run. This permits arbitrary code injection into the CGI process on Linux.

To exploit the vulnerability, an attacker would create a HTTP CGI request that sets LD_PRELOAD=/proc/self/fd/0 in the query string and sets the POST data of the request to be in the form of a malicious shared library for the architecture of the device.

Here is an isolated patch that can be applied immediately.

diff --git a/src/cgi.c b/src/cgi.c
index 899ec97..18d9b45 100644
--- a/src/cgi.c
+++ b/src/cgi.c
@@ -160,10 +160,17 @@ PUBLIC bool cgiHandler(Webs *wp)
     envpsize = 64;
     envp = walloc(envpsize * sizeof(char*));
     for (n = 0, s = hashFirst(wp->vars); s != NULL; s = hashNext(wp->vars, s)) {
-        if (s->content.valid && s->content.type == string &&
-            strcmp(s->name.value.string, "REMOTE_HOST") != 0 &&
-            strcmp(s->name.value.string, "HTTP_AUTHORIZATION") != 0) {
-            envp[n++] = sfmt("%s=%s", s->name.value.string, s->content.value.string);
+        if (s->content.valid && s->content.type == string) {
+            if (smatch(s->name.value.string, "REMOTE_HOST") ||
+                smatch(s->name.value.string, "HTTP_AUTHORIZATION") ||
+                smatch(s->name.value.string, "IFS") ||
+                smatch(s->name.value.string, "CDPATH") ||
+                smatch(s->name.value.string, "PATH") ||
+                sstarts(s->name.value.string, "LD_")) {
+                continue;
+            }
@mobrien

This comment has been minimized.

Show comment
Hide comment
@mobrien

mobrien Jun 9, 2017

Contributor

As part of a more general and robust solution, a new main.me property "cgiVarPrefix" is added in 3.6.5.

When the prefix is set to “CGI_” (the default), all user supplied query and form variables are prefixed with CGI_. This will require changes to your CGI programs to expect this prefix. It is recommended that users do this.

Contributor

mobrien commented Jun 9, 2017

As part of a more general and robust solution, a new main.me property "cgiVarPrefix" is added in 3.6.5.

When the prefix is set to “CGI_” (the default), all user supplied query and form variables are prefixed with CGI_. This will require changes to your CGI programs to expect this prefix. It is recommended that users do this.

@mobrien mobrien closed this Jun 9, 2017

@mobrien mobrien referenced this issue Dec 26, 2017

Closed

CVE-2017-17562 #262

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment