jvoisin / snuffleupagus

Security module for php7 and php8 - Killing bugclasses and virtual-patching the rest!
https://snuffleupagus.readthedocs.io
GNU Lesser General Public License v3.0
765 stars 88 forks source link

Invalid line reporting when wrong configuration file is used #306

Open xXx-caillou-xXx opened 5 years ago

xXx-caillou-xXx commented 5 years ago

Hi, It seems like there is an error in the reporting of invalid configuration.

Using this configuration file:

# This
# is
# a
# comment
sp.does_not_exist.enable();

I get:

PHP message: PHP Fatal error:  [snuffleupagus][config] Invalid configuration file in Unknown on line 0

I'm using the latest version of master, with php7.3-fpm. I can do a PR for this, but I currently don't have the time to do so.

I think this could be done at the same time as #300

jvoisin commented 5 years ago

This should fail on startup, while is it failing on runtime?

I'll try to work on this this month, but since I'm moving into a new flat, I don't have much free time at the moment :)

kkadosh commented 5 years ago

The problem is not from snuffleupagus. As you can see here, snuffleupagus only log Invalid configuration file , so the in Unknown on line 0 is appended by something else.

Furthermore, snuffleupagus logs correctly the line here but also append the in Unknown on line 0 as you can see below in the test i did (with the same broken configuration file that @xXx-caillou-xXx provided):

Fatal error: [snuffleupagus][config] Invalid configuration section 'sp.does_not_exist.enable();' on line 5 in Unknown on line 0

Fatal error: [snuffleupagus][config] Invalid configuration file in Unknown on line 0
Could not startup.

We can see that the line 5 is correctly mentionned.

I will try to look for what appending this "in Unknown line 0" (its looks like its PHP that add this and might be related to https://github.com/nbs-system/snuffleupagus/issues/307)

kkadosh commented 5 years ago

After some debug, it looks like the problem come from PHP. Indeed, in sp_log_msg() we use zend_error().

This function then use get_filename_lineno() which set the filename to Unknown and the line to 0.

@jvoisin @xXx-caillou-xXx , do you have any idea how can we handle/fix this ?

dkowis commented 2 years ago

I just ran into this again, and it's making it super difficult to debug my invalid configuration. I don't honestly know what changed that made the configuration invalid either :(

I've used the default config from this repo, and sadly, it's not working.

bef commented 2 years ago
Fatal error: [snuffleupagus][0.0.0.0][config][log] Unexpected keyword 'does_not_exist' on line 5 in Unknown on line 0

Fatal error: [snuffleupagus][0.0.0.0][config][log] Invalid configuration file in Unknown on line 0
Could not startup.

The error message looks just detailed enough for me to find an invalid keyword in line 5 of the configuration file. If you do not see a message like this with a syntactically invalid configuration file, please try the all new release version 0.8.2.