Project

General

Profile

Actions

Bug #2798

closed

Segfault with simple-vhost.debug = "enable"

Added by avij about 7 years ago. Updated about 7 years ago.

Status:
Fixed
Priority:
Normal
Category:
mod_simple_vhost
Target version:
ASK QUESTIONS IN Forums:

Description

In issue #2797 it was discovered that simple-vhost.debug takes an integer and not a string. Now that I had a look at my /var/log/messages I saw this: lighttpd25802: segfault at 10 ip 00007fa083a8ddd0 sp 00007ffe4ddb0f40 error 4 in mod_simple_vhost.so[7fa083a8d000+2000]

I hadn't noticed the segfault at the time, but upon further examination I noticed that having simple-vhost.debug = "enable" in my simple_vhost.conf was indeed the cause for the segfault. This is probably rather harmless as lighttpd won't start with an invalid config anyway, but I still believe programs should not crash even if given invalid input. A configuration file exhibiting this problem is attached.

What I've found so far is that with my config, srv->config_context->used seems to be set to 2, but the plugin_config at index 1 is null. I'm unsure which is incorrect. The segfault happens at buffer_free(s->document_root).

Initial testing was done with lighttpd 1.4.45, but I was able to reproduce this with the version in git as of today.


Files

config.conf (29.7 KB) config.conf lighttpd configuration avij, 2017-03-13 21:50
Actions #1

Updated by gstrauss about 7 years ago

This is probably rather harmless as lighttpd won't start with an invalid config anyway, but I still believe programs should not crash even if given invalid input.

I agree.

I was unable to reproduce with tip of lighttpd git master using:

    server.port                    = 8080
    server.document-root           = "/dev/shm" 
    server.modules                 = ("mod_simple_vhost")

    simple-vhost.server-root       = "/var/www/vhosts/" 
    simple-vhost.default-host      = "default.example.com" 
    simple-vhost.document-root     = "/dev/shm/" 
    simple-vhost.debug             = "enable" 

2017-03-13 18:31:21: (server.c.1277) server started (lighttpd/1.4.46-devel-lighttpd-1.4.45-79-g46ff978) 
2017-03-13 18:31:21: (configfile-glue.c.113) got a string but expected a short: simple-vhost.debug enable 
2017-03-13 18:31:21: (server.c.1285) Configuration of plugins failed. Going down.

Please check if you can reproduce this if you do a clean build, including "./autogen.sh; ./configure ..." before the build.

Actions #2

Updated by gstrauss about 7 years ago

Appears there is a missing check in mod_simple_vhost.c:mod_simple_vhost_free(), and is present in other modules.

--- a/src/mod_simple_vhost.c
+++ b/src/mod_simple_vhost.c
@@ -55,6 +55,7 @@ FREE_FUNC(mod_simple_vhost_free) {
                size_t i;
                for (i = 0; i < srv->config_context->used; i++) {
                        plugin_config *s = p->config_storage[i];
+                       if (NULL == s) continue;

                        buffer_free(s->document_root);
                        buffer_free(s->default_host);

Actions #3

Updated by avij about 7 years ago

I'm not exactly sure which combination triggers the segfault, but:

[root@c7build sbin]# ./lighttpd -f config.conf -D
Segmentation fault

[root@c7build sbin]# ./lighttpd -f gstrauss.conf -D
2017-03-14 00:40:56: (server.c.1277) server started (lighttpd/1.4.46-devel-lighttpd-1.4.45-79-g46ff978)
2017-03-14 00:40:56: (configfile-glue.c.113) got a string but expected a short: simple-vhost.debug enable
2017-03-14 00:40:56: (server.c.1285) Configuration of plugins failed. Going down.

where config.conf is the attached config file, and gstrauss.conf is the config you gave above. In any case, the segfault is gone with the patch. Thanks :)

Actions #4

Updated by gstrauss about 7 years ago

  • Status changed from New to Patch Pending
  • Target version changed from 1.4.x to 1.4.46

If srv->config_context->used is incremented, but modules fail to be initialized, then calling the free function on another module may result in running cleanup on per-module contexts that might not yet have been initialized. This was also a problem in mod_openssl and mod_status.

As you noted, this is not a security issue since the crash is with an invalid config at startup, but still something to be fixed.

Actions #5

Updated by gstrauss about 7 years ago

FYI: before patch, will crash with something that adds a per-context config element:

    server.port                    = 8080
    server.document-root           = "/dev/shm" 
    server.modules                 = ("mod_simple_vhost")

    simple-vhost.server-root       = "/var/www/vhosts/" 
    simple-vhost.default-host      = "default.example.com" 
    simple-vhost.document-root     = "/dev/shm/" 
    simple-vhost.debug             = "enable" 

    $HTTP["url"] =~ "\.pdf$" {
        server.range-requests = "disable" 
    }

Actions #6

Updated by avij about 7 years ago

gstrauss wrote:

Appears there is a missing check in mod_simple_vhost.c:mod_simple_vhost_free(), and is present in other modules.

mod_status.c might also be affected?

edit: ok, you've noticed this already.

Actions #7

Updated by gstrauss about 7 years ago

  • Status changed from Patch Pending to Fixed
  • % Done changed from 0 to 100
Actions

Also available in: Atom