Project

General

Profile

Bug #2798

Segfault with simple-vhost.debug = "enable"

Added by avij 6 months ago. Updated 6 months ago.

Status:
Fixed
Priority:
Normal
Assignee:
-
Category:
mod_simple_vhost
Target version:
Start date:
2017-03-13
Due date:
% Done:

100%

Estimated time:
Missing in 1.5.x:

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.

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

Associated revisions

Revision 1485cb40 (diff)
Added by gstrauss 6 months ago

[core] fix crash if invalid config file (fixes #2798)

If lighttpd.conf is invalid, some modules may not have initialized their
per-context config structures, but will have their free-functions
called, which should not be run on uninitialized per-context configs.

x-ref:
"Segfault with simple-vhost.debug = "enable""
https://redmine.lighttpd.net/issues/2798

History

#1 Updated by gstrauss 6 months 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.

#2 Updated by gstrauss 6 months 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);

#3 Updated by avij 6 months 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 :)

#4 Updated by gstrauss 6 months 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.

#5 Updated by gstrauss 6 months 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" 
    }

#6 Updated by avij 6 months 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.

#7 Updated by gstrauss 6 months ago

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

Also available in: Atom