Skip to content
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

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

Open
NewEraCracker opened this issue Aug 13, 2014 · 7 comments
Labels

Comments

@NewEraCracker
Copy link
Contributor

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
Copy link
Collaborator

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

@bef
Copy link
Member

bef commented Oct 16, 2014

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

@NewEraCracker
Copy link
Contributor Author

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
Copy link
Member

bef commented Oct 17, 2014

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

@NewEraCracker
Copy link
Contributor Author

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

@bef bef added the windows label Nov 20, 2014
@NewEraCracker
Copy link
Contributor Author

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
Copy link
Contributor Author

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;
 }

NewEraCracker referenced this issue in NewEraCracker/suhosin Apr 8, 2016
Fix 'Suhosin fails to correctly override post handles when Apache SAPI is used'

Also, while I'm here, clean up unused code and old comments
NewEraCracker referenced this issue in NewEraCracker/suhosin Jul 31, 2016
Fix 'Suhosin fails to correctly override post handles when Apache SAPI is used'

Also, while I'm here, clean up unused code and old comments
NewEraCracker referenced this issue in NewEraCracker/suhosin Aug 26, 2016
Fix 'Suhosin fails to correctly override post handles when Apache SAPI is used'

Also, while I'm here, clean up unused code and old comments
NewEraCracker referenced this issue in NewEraCracker/suhosin Sep 29, 2016
Fix 'Suhosin fails to correctly override post handles when Apache SAPI is used'

Also, while I'm here, clean up unused code and old comments
NewEraCracker referenced this issue in NewEraCracker/suhosin Oct 7, 2016
Fix 'Suhosin fails to correctly override post handles when Apache SAPI is used'

Also, while I'm here, clean up unused code and old comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants