Suhosin fails to correctly override post handles when Apache SAPI is used

[NewEraCracker]

Post handlers are not replaced when Apache SAPI is used. I think this must be due module start order being Zend > Modules > SAPI (unconfirmed).

The fix for this problem is to change the hooking position to the activate stage (that is run after the startup stage).

I have done this patch and confirm ELF uploads are successfully intercepted and dropped in both Apache SAPI and CGI SAPI (the latter does work, even without this patch).

diff -uNra suhosin-0.9.36/suhosin.c suhosin-0.9.36.new/suhosin.c
--- suhosin-0.9.36/suhosin.c    Tue Jun 10 09:58:36 2014
+++ suhosin-0.9.36.new/suhosin.c    Wed Aug 13 18:14:07 2014
@@ -46,18 +46,19 @@
 static int (*old_startup)(zend_extension *extension) = NULL;
 static zend_extension *ze = NULL;

-static int suhosin_module_startup(zend_extension *extension);
-static void suhosin_shutdown(zend_extension *extension);
-
-
+static void (*orig_module_activate)(void) = NULL;
+static void (*orig_module_deactivate)(void) = NULL;
 static void (*orig_op_array_ctor)(zend_op_array *op_array) = NULL;
 static void (*orig_op_array_dtor)(zend_op_array *op_array) = NULL;
 static void (*orig_module_shutdown)(zend_extension *extension) = NULL;
 static int (*orig_module_startup)(zend_extension *extension) = NULL;

-
+static void suhosin_module_activate(void);
+static void suhosin_module_deactivate(void);
 static void suhosin_op_array_ctor(zend_op_array *op_array);
 static void suhosin_op_array_dtor(zend_op_array *op_array);
+static void suhosin_shutdown(zend_extension *extension);
+static int  suhosin_module_startup(zend_extension *extension);

 STATIC zend_extension suhosin_zend_extension_entry = {
    "Suhosin",
@@ -67,8 +68,8 @@
    "Copyright (c) 2007-2014",
    suhosin_module_startup,
    suhosin_shutdown,
-   NULL,
-   NULL,
+   suhosin_module_activate,
+   suhosin_module_deactivate,
    NULL,
    NULL,
    NULL,
@@ -80,6 +81,20 @@
    STANDARD_ZEND_EXTENSION_PROPERTIES
 };

+static void suhosin_module_activate(void)
+{
+   TSRMLS_FETCH();
+
+   suhosin_hook_post_handlers(TSRMLS_C);
+}
+
+static void suhosin_module_deactivate(void)
+{
+   TSRMLS_FETCH();
+
+   suhosin_unhook_post_handlers(TSRMLS_C);
+}
+
 static void suhosin_op_array_ctor(zend_op_array *op_array)
 {
    TSRMLS_FETCH();
@@ -108,6 +123,22 @@

 /* Stealth Mode functions */

+static void stealth_module_activate(void)
+{
+   if (orig_module_activate != NULL) {
+       orig_module_activate();
+   }
+   suhosin_module_activate();
+}
+
+static void stealth_module_deactivate(void)
+{
+   if (orig_module_deactivate != NULL) {
+       orig_module_deactivate();
+   }
+   suhosin_module_deactivate();
+}
+
 static void stealth_op_array_ctor(zend_op_array *op_array)
 {
    if (orig_op_array_ctor != NULL) {
@@ -146,8 +177,6 @@
    int resid;
    TSRMLS_FETCH();

-/* zend_register_module(&suhosin_module_entry TSRMLS_CC); */
-   
    if (zend_hash_find(&module_registry, "suhosin", sizeof("suhosin"), (void **)&module_entry_ptr)==SUCCESS) {

        if (extension) {
@@ -156,10 +185,7 @@
            zend_extension ext;
            ext = suhosin_zend_extension_entry;
            ext.handle = module_entry_ptr->handle;
-           /*
-           zend_llist_add_element(&zend_extensions, &ext);
-           extension = zend_llist_get_last(&zend_extensions);
-           */
+
            extension = &suhosin_zend_extension_entry;
        }
        module_entry_ptr->handle = NULL;
@@ -177,7 +203,6 @@
    suhosin_zend_extension_entry.resource_number = resid;

    suhosin_hook_treat_data();
-   suhosin_hook_post_handlers(TSRMLS_C);
    suhosin_aes_gentables();
    suhosin_hook_register_server_variables();
    suhosin_hook_header_handler();
@@ -191,20 +216,18 @@

 static void suhosin_shutdown(zend_extension *extension)
 {
-   TSRMLS_FETCH();
-
    suhosin_unhook_execute();
    suhosin_unhook_header_handler();
-   suhosin_unhook_post_handlers(TSRMLS_C);
    /* suhosin_unhook_session(); - enabling this causes compability problems */

     if (ze != NULL) {
        ze->startup = orig_module_startup;
        ze->shutdown = orig_module_shutdown;
+       ze->activate = orig_module_activate;
+       ze->deactivate = orig_module_deactivate;
        ze->op_array_ctor = orig_op_array_ctor;
        ze->op_array_dtor = orig_op_array_dtor;
     }
-    
 }


@@ -214,7 +237,6 @@
    zend_extension *ex = &suhosin_zend_extension_entry;
    char *new_info;
    int new_info_length;
-   TSRMLS_FETCH();

    /* Ugly but working hack */
    new_info_length = sizeof("%s\n    with %s v%s, %s, by %s\n")
@@ -233,28 +255,22 @@
    /* Stealth Mode */
    orig_module_startup = ze->startup;
    orig_module_shutdown = ze->shutdown;
+   orig_module_activate = ze->activate;
+   orig_module_deactivate = ze->deactivate;
    orig_op_array_ctor = ze->op_array_ctor;
    orig_op_array_dtor = ze->op_array_dtor;

-    /*if (SUHOSIN_G(stealth) != 0) {*/
-       ze->startup = stealth_module_startup;
-       ze->shutdown = stealth_module_shutdown;
-       ze->op_array_ctor = stealth_op_array_ctor;
-       ze->op_array_dtor = stealth_op_array_dtor;
-    /*}*/
+   ze->startup = stealth_module_startup;
+   ze->shutdown = stealth_module_shutdown;
+   ze->activate = stealth_module_activate;
+   ze->deactivate = stealth_module_deactivate;
+   ze->op_array_ctor = stealth_op_array_ctor;
+   ze->op_array_dtor = stealth_op_array_dtor;

    res = old_startup(ext);

-/*    ex->name = NULL; 
-    ex->author = NULL;
-    ex->copyright = NULL;
-    ex->version = NULL;*/
-
-    /*zend_extensions.head=NULL;*/
-
    suhosin_module_startup(NULL);

-   
    return res;
 }

Regards, NewEraCracker

[stefanesser]

We will look into this during this week and release an update after that.

[bef]

This issue can not be reproduced. Please provide Apache + PHP version numbers.

[NewEraCracker]

I remember I could reproduce this with PHP running as Apache module. Not sure if this is Windows specific but for the record I could reproduce this with Apache 2.4 and PHP 5.2 (Suhosin 0.9.33), PHP 5.3 to 5.5 (Suhosin 0.9.36) so this is not version specific.

[bef]

Have you encountered this issue on a system other than windows as well?

[NewEraCracker]

I will have to setup a Linux box to find out. Will report later if I encounter the issue there or not.

[NewEraCracker]

Just an heads up.

I've tested latest Suhosin master and PHP 5.4.36-0+deb7u3 in Kali Linux (a Debian derivative) and was unable to reproduce.

Yet, I have tested in Windows with latest master and PHP 5.4.38 and I am still able to reproduce this problem while using PHP with Apache 2.4 SAPI.

Regards, NewEraCracker

[NewEraCracker]

Applying an adapted version of the previous patch fixes the problem.

--- suhosin.c
+++ suhosin.c
@@ -50,13 +50,15 @@
 static int suhosin_module_startup(zend_extension *extension);
 static void suhosin_shutdown(zend_extension *extension);

-
+static void (*orig_module_activate)(void) = NULL;
+static void (*orig_module_deactivate)(void) = NULL;
 static void (*orig_op_array_ctor)(zend_op_array *op_array) = NULL;
 static void (*orig_op_array_dtor)(zend_op_array *op_array) = NULL;
 static void (*orig_module_shutdown)(zend_extension *extension) = NULL;
 static int (*orig_module_startup)(zend_extension *extension) = NULL;

-
+static void suhosin_module_activate(void);
+static void suhosin_module_deactivate(void);
 static void suhosin_op_array_ctor(zend_op_array *op_array);
 static void suhosin_op_array_dtor(zend_op_array *op_array);

@@ -68,8 +70,8 @@
    "Copyright (c) 2007-2015",
    suhosin_module_startup,
    suhosin_shutdown,
-   NULL,
-   NULL,
+   suhosin_module_activate,
+   suhosin_module_deactivate,
    NULL,
    NULL,
    NULL,
@@ -81,6 +83,20 @@
    STANDARD_ZEND_EXTENSION_PROPERTIES
 };

+static void suhosin_module_activate(void)
+{
+   TSRMLS_FETCH();
+
+   suhosin_hook_post_handlers(TSRMLS_C);
+}
+
+static void suhosin_module_deactivate(void)
+{
+   TSRMLS_FETCH();
+
+   suhosin_unhook_post_handlers(TSRMLS_C);
+}
+
 static void suhosin_op_array_ctor(zend_op_array *op_array)
 {
    TSRMLS_FETCH();
@@ -109,6 +125,22 @@

 /* Stealth Mode functions */

+static void stealth_module_activate(void)
+{
+   if (orig_module_activate != NULL) {
+       orig_module_activate();
+   }
+   suhosin_module_activate();
+}
+
+static void stealth_module_deactivate(void)
+{
+   if (orig_module_deactivate != NULL) {
+       orig_module_deactivate();
+   }
+   suhosin_module_deactivate();
+}
+
 static void stealth_op_array_ctor(zend_op_array *op_array)
 {
    if (orig_op_array_ctor != NULL) {
@@ -147,8 +179,6 @@
    int resid;
    TSRMLS_FETCH();

-/* zend_register_module(&suhosin_module_entry TSRMLS_CC); */
-   
    if (zend_hash_find(&module_registry, "suhosin", sizeof("suhosin"), (void **)&module_entry_ptr)==SUCCESS) {

        if (extension) {
@@ -157,10 +187,7 @@
            zend_extension ext;
            ext = suhosin_zend_extension_entry;
            ext.handle = module_entry_ptr->handle;
-           /*
-           zend_llist_add_element(&zend_extensions, &ext);
-           extension = zend_llist_get_last(&zend_extensions);
-           */
+
            extension = &suhosin_zend_extension_entry;
        }
        module_entry_ptr->handle = NULL;
@@ -178,7 +205,6 @@
    suhosin_zend_extension_entry.resource_number = resid;

    suhosin_hook_treat_data();
-   suhosin_hook_post_handlers(TSRMLS_C);
    suhosin_aes_gentables();
    suhosin_hook_register_server_variables();
    suhosin_hook_header_handler();
@@ -192,20 +218,18 @@

 static void suhosin_shutdown(zend_extension *extension)
 {
-   TSRMLS_FETCH();
-
    suhosin_unhook_execute();
    suhosin_unhook_header_handler();
-   suhosin_unhook_post_handlers(TSRMLS_C);
    /* suhosin_unhook_session(); - enabling this causes compability problems */

     if (ze != NULL) {
        ze->startup = orig_module_startup;
        ze->shutdown = orig_module_shutdown;
+       ze->activate = orig_module_activate;
+       ze->deactivate = orig_module_deactivate;
        ze->op_array_ctor = orig_op_array_ctor;
        ze->op_array_dtor = orig_op_array_dtor;
     }
-    
 }


@@ -215,7 +239,6 @@
    zend_extension *ex = &suhosin_zend_extension_entry;
    char *new_info;
    int new_info_length;
-   TSRMLS_FETCH();

    /* Ugly but working hack */
    new_info_length = sizeof("%s\n    with %s v%s, %s, by %s\n")
@@ -234,30 +257,24 @@
    /* Stealth Mode */
    orig_module_startup = ze->startup;
    orig_module_shutdown = ze->shutdown;
+   orig_module_activate = ze->activate;
+   orig_module_deactivate = ze->deactivate;
    orig_op_array_ctor = ze->op_array_ctor;
    orig_op_array_dtor = ze->op_array_dtor;

-    /*if (SUHOSIN_G(stealth) != 0) {*/
-       ze->startup = stealth_module_startup;
-       ze->shutdown = stealth_module_shutdown;
-       ze->op_array_ctor = stealth_op_array_ctor;
-       ze->op_array_dtor = stealth_op_array_dtor;
-    /*}*/
+   ze->startup = stealth_module_startup;
+   ze->shutdown = stealth_module_shutdown;
+   ze->activate = stealth_module_activate;
+   ze->deactivate = stealth_module_deactivate;
+   ze->op_array_ctor = stealth_op_array_ctor;
+   ze->op_array_dtor = stealth_op_array_dtor;

    if (old_startup != NULL) {
        res = old_startup(ext);
    }

-/*    ex->name = NULL; 
-    ex->author = NULL;
-    ex->copyright = NULL;
-    ex->version = NULL;*/
-
-    /*zend_extensions.head=NULL;*/
-
    suhosin_module_startup(NULL);
-    
-   
+
    return res;
 }