pluginsGLPI / customfields

customfields plugin for GLPI
GNU General Public License v2.0
13 stars 4 forks source link

'Security die. trying to load an forbidden class name' in Historical after upgrading (and editing) data #79

Open marcogaio opened 8 years ago

marcogaio commented 8 years ago

I've upgraded to GLPI 0.85.X and upgrade the plugin to latest (master branch).

For 'Devices' plugin works, but IF I ADD/MODIFY customfield data, the 'historical' tab stop working, displaying the error 'Security die. trying to load an forbidden class name'.

Nothing get to logs (GLPI or Apache errorlog).

Thanks.

btry commented 8 years ago

Hi

There are 2 classes in GLPI 0.72.x which are obsolete in GLPI 0.85. When you upgrade GLPI, the files for these classes are not cleaned up. Deleting them solves the issue.

An other possible cause : in old versions of GLPI, itemtypes where numbers. Some of then are not migrated because I could not find which new itemtypes (names) match.

I'll give you some directives soon to find out what happens and how to solve the issue.

marcogaio commented 8 years ago

Ahem, sorry, i'm still seeking feedback.

The proposed solution in list glpi-user does not work for me, look at https://mail.gna.org/public/glpi-user/2015-11/msg00010.html .

Thanks.

btry commented 8 years ago

Hi, Sorry for the delay.

First, use a testing environment and backup it to easily go back if needed.

Check there are no numeric values in the column itemtype of the table glpi_plugin_customfields_itemtypes. If you got such values then I guess you were using Custom Fields since GLPI 0.72 or older. It you never lost custom fields data when you upgraded to GLPI 0.84 or later, then you should delete these lines.

Check the historical tab works again and feedback.

marcogaio commented 8 years ago

glpi_plugin_customfields_itemtypes.itemtype have no numerical data.

Effectively, i'm using 'customfield' plugin from early GLPI release...

btry commented 8 years ago

Do the security die occur only when you want to display an historical tab ? If not, please tell me the other cases.

marcogaio commented 8 years ago

Only for historical. And only for some item (some peripherals, some computers, ...).

btry commented 8 years ago

Since GLPI 0.85.x there is a check to disallow loading classes named with only digits. I think your glpi_logs table has some rows with a numeric value in the column itemtype.

glpi_logs may be a very big table, and the column itemtype is not indexed a varchar. You may end to a high CPU load if you choose a poorly optimised search method.

check there is no numeric value of itemtype, very likely in the range 501; 512

marcogaio commented 8 years ago

Seems no:

mysql> select * from glpi_logs where itemtype regexp '^[0-9]*$';
Empty set (29.77 sec)
btry commented 8 years ago

You said this affects only some items.

Can you query your database for the historical of two items : one having a display issue, and an other not having this issue (same itemtype would be best).

I don't know exactly what you should find, most likely an inconsistency related to itemtypes.

You may also hack GLPI to change the security die message, to show which class name is forbidden. This may give a good clue about this bug.

marcogaio commented 8 years ago

OK, done.

mysql> select * from glpi_logs where itemtype='Peripheral' and items_id=536;
+----------+------------+----------+---------------+---------------+------------------------------+---------------------+------------------+-----------+-----------+
| id       | itemtype   | items_id | itemtype_link | linked_action | user_name                    | date_mod            | id_search_option | old_value | new_value |
+----------+------------+----------+---------------+---------------+------------------------------+---------------------+------------------+-----------+-----------+
| 34960873 | Peripheral |      536 | 0             |            20 | Plugin_FusionInventory (331) | 2015-10-27 10:54:52 |                0 |           |           |
+----------+------------+----------+---------------+---------------+------------------------------+---------------------+------------------+-----------+-----------+
1 row in set (0.00 sec)

mysql> select * from glpi_logs where itemtype='Peripheral' and items_id=537;
+----------+------------+----------+---------------------------+---------------+-------------------+---------------------+------------------+------------+---------------------------------------------------------------------------------+
| id       | itemtype   | items_id | itemtype_link             | linked_action | user_name         | date_mod            | id_search_option | old_value  | new_value                                                                       |
+----------+------------+----------+---------------------------+---------------+-------------------+---------------------+------------------+------------+---------------------------------------------------------------------------------+
| 34961054 | Peripheral |      537 | 0                         |            20 | Marco Gaiarin (8) | 2015-10-27 14:36:15 |                0 |            |                                                                                 |
| 34961056 | Peripheral |      537 | 0                         |            18 | Marco Gaiarin (8) | 2015-10-27 14:36:28 |                0 | civab ()   | civab (--- --- --)                                                              |
| 34961057 | Peripheral |      537 | 0                         |            18 | Marco Gaiarin (8) | 2015-10-27 14:36:28 |                0 | ec (0)     | ec (1)                                                                          |
| 34961058 | Peripheral |      537 | 0                         |            18 | Marco Gaiarin (8) | 2015-10-27 14:36:28 |                0 | vse (0)    | vse (1)                                                                         |
| 34961060 | Peripheral |      537 | PluginAppliancesAppliance |            15 | Marco Gaiarin (8) | 2015-10-27 14:36:36 |                0 |            | s-emg-3 (28)                                                                    |
| 34961061 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-27 14:36:54 |                3 |   (0) | San Vito > Edificio Principale > Primo Piano > 38 - Elettroencefalografia (273) |
| 34961062 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-27 14:36:54 |               71 |   (0) | tnfp (65)                                                                       |
| 34963413 | Peripheral |      537 | Document                  |            15 | Marco Gaiarin (8) | 2015-10-28 15:25:52 |                0 |            | Documento: Periferica - tdi-13 (2545)                                           |
| 34963463 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |               53 |   (0) | Officina Biomedica  (244)                                                       |
| 34963464 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |              121 |            | 2015-10-23                                                                      |
| 34963465 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |               27 |            | 15/0877                                                                         |
| 34963466 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |               38 |            | 2015-10-23                                                                      |
| 34963467 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |              123 |            | 2015-10-23                                                                      |
| 34963468 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |               51 | 0          | 12                                                                              |
+----------+------------+----------+---------------------------+---------------+-------------------+---------------------+------------------+------------+---------------------------------------------------------------------------------+
14 rows in set (0.00 sec)

I've done:

update glpi_logs set itemtype_link='' where itemtype='Peripheral' and items_id=537 and itemtype_link='0';

and logs reappear. Wonderful!

Now, known the trouble, i've done the query:

select id from glpi_logs where itemtype_link regexp '^[0-9]+$';

That result effectively in TONS of rows. It is safer to set also that to ''?!

marcogaio commented 8 years ago

Sorry, a little more note.

I've tried to modify some customfiled, and error reappear, so seems thta the plugin insert the row with '0' and not ''.

mysql> select * from glpi_logs where itemtype='Peripheral' and items_id=537;
+----------+------------+----------+---------------------------+---------------+-------------------+---------------------+------------------+------------+---------------------------------------------------------------------------------+
| id       | itemtype   | items_id | itemtype_link             | linked_action | user_name         | date_mod            | id_search_option | old_value  | new_value                                                                       |
+----------+------------+----------+---------------------------+---------------+-------------------+---------------------+------------------+------------+---------------------------------------------------------------------------------+
| 34961054 | Peripheral |      537 |                           |            20 | Marco Gaiarin (8) | 2015-10-27 14:36:15 |                0 |            |                                                                                 |
| 34961056 | Peripheral |      537 |                           |            18 | Marco Gaiarin (8) | 2015-10-27 14:36:28 |                0 | civab ()   | civab (--- --- --)                                                              |
| 34961057 | Peripheral |      537 |                           |            18 | Marco Gaiarin (8) | 2015-10-27 14:36:28 |                0 | ec (0)     | ec (1)                                                                          |
| 34961058 | Peripheral |      537 |                           |            18 | Marco Gaiarin (8) | 2015-10-27 14:36:28 |                0 | vse (0)    | vse (1)                                                                         |
| 34961060 | Peripheral |      537 | PluginAppliancesAppliance |            15 | Marco Gaiarin (8) | 2015-10-27 14:36:36 |                0 |            | s-emg-3 (28)                                                                    |
| 34961061 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-27 14:36:54 |                3 |   (0) | San Vito > Edificio Principale > Primo Piano > 38 - Elettroencefalografia (273) |
| 34961062 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-27 14:36:54 |               71 |   (0) | tnfp (65)                                                                       |
| 34963413 | Peripheral |      537 | Document                  |            15 | Marco Gaiarin (8) | 2015-10-28 15:25:52 |                0 |            | Documento: Periferica - tdi-13 (2545)                                           |
| 34963463 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |               53 |   (0) | Officina Biomedica  (244)                                                       |
| 34963464 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |              121 |            | 2015-10-23                                                                      |
| 34963465 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |               27 |            | 15/0877                                                                         |
| 34963466 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |               38 |            | 2015-10-23                                                                      |
| 34963467 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |              123 |            | 2015-10-23                                                                      |
| 34963468 | Peripheral |      537 |                           |             0 | Marco Gaiarin (8) | 2015-10-28 16:17:39 |               51 | 0          | 12                                                                              |
| 48729538 | Peripheral |      537 | 0                         |            18 | Marco Gaiarin (8) | 2015-11-30 17:40:47 |                0 | nvp ()     | nvp (ciao)                                                                      |
| 48730587 | Peripheral |      537 | 0                         |            18 | Marco Gaiarin (8) | 2015-11-30 17:40:57 |                0 | nvp (ciao) | nvp ()                                                                          |
+----------+------------+----------+---------------------------+---------------+-------------------+---------------------+------------------+------------+---------------------------------------------------------------------------------+
16 rows in set (0.00 sec)
btry commented 8 years ago

I checked the code, the plugin should insert a log entry with itemtype_link = ''.

Can you compare customfields/inc/field.class.php, function post_updateItem() with the code in repository ?

Would be interesting to check which class name trigers the error. In glpi/inc/autoload.function.php, find "Security die" and change the message to show $classname.

marcogaio commented 8 years ago
root@armitage:~# LANG=C diff -ud /usr/local/share/glpi/plugins/customfields/inc/field.class.php /tmp/field.class.php
--- /usr/local/share/glpi/plugins/customfields/inc/field.class.php  2014-05-04 15:01:59.000000000 +0200
+++ /tmp/field.class.php    2015-12-01 12:30:14.713503576 +0100
@@ -490,7 +490,7 @@
                $oldvalues,
                $newvalues
             ),
-            0, 
+            "", 
             Log::HISTORY_UPDATE_SUBITEM
          );

@@ -498,4 +498,4 @@

    }

-}
\ No newline at end of file
+}

Bug found. ;-)

For the second question: With a patch like that:

root@armitage:~# diff -ud /usr/local/share/glpi/inc/autoload.function.php.orig /usr/local/share/glpi/inc/autoload.function.php
--- /usr/local/share/glpi/inc/autoload.function.php.orig    2015-09-15 18:10:47.000000000 +0200
+++ /usr/local/share/glpi/inc/autoload.function.php 2015-12-01 12:32:31.615163236 +0100
@@ -261,7 +261,7 @@

    // empty classname or non concerted plugin or classname containing dot (leaving GLPI main treee)
    if (empty($classname) || is_numeric($classname) || (strpos($classname, '.') !== false)) {
-      die("Security die. trying to load an forbidden class name");
+      die("Security die. trying to load an forbidden class name ($classname)");
    }

GLPI now print:

Security die. trying to load an forbidden class name (0)

So seems to me that all things now are OK. Sorry, probably i've missed some commits, considering customfield now ''dead''.

btry commented 8 years ago

Strange... I don't remember any commit related to history for more than a year, the last I can remember was for GLPI 0.84.x .

I'm not sure what you mean by "Custom Fields is "dead". If this is because your history contains many 0 instead '', I think a update query will be harmless.

The worst such a query can do is to erase some meta data about the history of assets, about the customfield's fields only. I think you may try first on a testing environment. Anyway, it is recommended to clean very old history entries. Therefore, after some time (mostly a few months) the impact of slight damaged entries will disappear. Do not forget to backup before doing it, anyway.

marcogaio commented 8 years ago

Boh... i sayed customfields is dead because there's no more development on it, moving to fields plugin. Probably i've downloaded an on version and supposing development is stalled, not upgraded anymore.

Thanks for the hints about cleaning the history!

btry commented 8 years ago

Hi

Actually this plugin reached its end of life, as stated a few weeks ago in the readme file.

Fields provides a feature Custom Fields does not provide and was frequently asked. Fields provides many new useful features. Therefore it is useless to maintain 2 similar plugins, and focusing on only one of them is obviously more efficient.

A migration script will be released (already available in the repo, but needs many more work, this is currently a not working yet draft).

If you're able to migrate to Fields by yourself, that's great :)

marcogaio commented 8 years ago

Probably i will migrate when i move to 0.90, still in planning... ;-)))