Laravel-Backpack / CRUD

Build custom admin panels. Fast!
https://backpackforlaravel.com
MIT License
3.15k stars 891 forks source link

[Bug] 500 Error prints raw SQL despite ENV=production/debug off #5653

Closed amenk closed 1 month ago

amenk commented 1 month ago

Bug report

What I did

Installed Laravel 11 and Backpack crud 6.7.25 on a host with MySQL5.6 With APP_DEBUG=false, APP_ENV=production

What I expected to happen

A clean error message, not revealing any technical details

What happened

![error message]https://github.com/user-attachments/assets/52b6449f-8316-434b-998b-e4dc986dd690)

What I've already tried to fix it

Could be fixed by removing the exception message in

vendor/backpack/crud/src/resources/views/ui/errors/500.blade.php:12

Repro if you don't want to install an old MySQL

You can also just break the SQL In

\Illuminate\Database\Schema\Grammars\MySqlGrammar::compileColumns

to force such an error to appear.

Is it a bug in the latest version of Backpack?

yes https://github.com/Laravel-Backpack/CRUD/blob/ae31f26dfe24ac72c94ae9a26069c104090cb894/src/resources/views/ui/errors/500.blade.php#L12

Backpack, Laravel, PHP, DB version

$ php artisan backpack:version
### PHP VERSION:
8.2.22

### PHP EXTENSIONS:
Core, date, libxml, openssl, pcre, zlib, filter, hash, json, pcntl, random, Reflection, SPL, session, standard, sodium, mysqlnd, PDO, xml, apcu, bcmath, calendar, ctype, curl, dom, mbstring, FFI, fileinfo, ftp, gd, gettext, iconv, imagick, intl, ldap, exif, mysqli, odbc, pdo_dblib, pdo_mysql, PDO_ODBC, pdo_sqlite, Phar, posix, readline, shmop, SimpleXML, soap, sockets, sqlite3, sysvmsg, sysvsem, sysvshm, tokenizer, xmlreader, xmlrpc, xmlwriter, xsl, zip, blackfire, Zend OPcache, xdebug

### LARAVEL VERSION:
11.19.0.0

### BACKPACK PACKAGE VERSIONS:
backpack/basset: 1.3.5
backpack/crud: 6.7.25
backpack/generators: v4.0.5
backpack/theme-coreuiv4: 1.1.2
backpack/theme-tabler: 1.2.11
welcome[bot] commented 1 month ago

Hello there! Thanks for opening your first issue on this repo!

Just a heads-up: Here at Backpack we use GitHub Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.

Backpack communication channels:

Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.

Thank you!

-- Justin Case The Backpack Robot

pxpm commented 1 month ago

Hey @amenk thanks for the report. Can you point me out how to reproduce this ?

I've tried to do as you described but I wasn't able to reproduce it. What I did ? Changed the SQL as you proposed to get the error.

// in \Illuminate\Database\Schema\Grammars\MySqlGrammar::compileColumns

public function compileColumns($database, $table)
    {
        return sprintf(
            'select column_name as `name`, data_type as `type_name`, column_type as `type`, '
            .'collation_name as `collation`, is_nullable as `nullable`, '
            .'column_default as `default`, column_comment as `comment`, '
            .'generation_expression as `expression`, extra as `extra` '
            .'from information_schema.columns where table_schema = %s and table_name = %s '
            .'order by ordinal_position asc',
            $this->quoteString($database),
-           $this->quoteString($table),
+          'lol'
        );
    }

With debug = false I got: image

With debug = true I got: image

I would like to reproduce this before taking any action. But I won't mind adding in our error views a check for app.debug to avoid such leaks.

{!! $exception?->getMessage() && config('app.debug') ? e($exception->getMessage()) : trans('backpack::base.error_page.message_500') !!}

Let me know, cheers.

amenk commented 1 month ago

Thanks for looking into this so quickly.

I change the query like this (prepending an x)

.'xgeneration_expression as expression, extra as extra '

The "lol" might break the sprintf instead of causing an SQL error which seems to lead to another error.

Also I am not sure what renders https://private-user-images.githubusercontent.com/7188159/364795406-a6fac359-ed25-46bb-93de-0cab5da35753.png - maybe some proxy might be involved in your setup and hides the error if the "xgeneration" does not reproduce it?

pxpm commented 1 month ago

Still unable to reproduce it. 😞

image

I am sure there is something different we both are doing. If I go to any controller, and I do an abort(500, 'test'), I get the Backpack error view: image

But if I break the query like you said, I get the exception shown in the picture.

Any idea of what I could be doing differently ?

Cheers

amenk commented 1 month ago

Strange. Which versions are you using? And the first screenshot does not look like APP_DEBUG=false + APP_ENV=production to me :-)

pxpm commented 1 month ago

Strange. Which versions are you using? And the first screenshot does not look like APP_DEBUG=false + APP_ENV=production to me :-)

Yes it's not with debug false, when debug => false, env => production I get the general 500 error from server: image

### PHP VERSION:
8.2.5

### PHP EXTENSIONS:
Core, bcmath, calendar, ctype, date, filter, hash, iconv, json, SPL, pcre, random, readline, Reflection, session, standard, mysqlnd, tokenizer, zlib, libxml, dom, PDO, openssl, SimpleXML, xml, xmlreader, xmlwriter, curl, ftp, fileinfo, gd, gmp, intl, mbstring, exif, mysqli, Phar, pdo_mysql, pdo_sqlite, sodium, sqlite3, zip, xsl, xdebug

### LARAVEL VERSION:
11.21.0.0

### BACKPACK PACKAGE VERSIONS:
backpack/activity-log: 2.0.5
backpack/backupmanager: v5.0.4
backpack/basset: 1.3.6
backpack/calendar-operation: 1.0.6
backpack/crud: 6.7.33
backpack/demo: dev-main
backpack/devtools: 3.1.6
backpack/editable-columns: 3.0.10
backpack/filemanager: 3.0.8
backpack/generators: v4.0.5
backpack/language-switcher: 2.0.0
backpack/logmanager: v5.0.2
backpack/medialibrary-uploaders: 1.2.0
backpack/menucrud: v4.0.1
backpack/newscrud: 5.1.0
backpack/pagemanager: 3.3.0
backpack/permissionmanager: 7.2.1
backpack/pro: 2.2.14
backpack/revise-operation: 2.0.0
backpack/settings: 3.1.1
backpack/theme-coreuiv2: 1.2.5
backpack/theme-coreuiv4: 1.1.2
backpack/theme-tabler: 1.2.12
backpack/translation-manager: 1.0.4

I will try to reproduce on a new installation and get back to you 👍

pxpm commented 1 month ago

Just tested on a new laravel + backpack + theme tabler (no addons).

Same result, if I mess with the query as you said, I either get the stack trace with debug => true image

or I get the 500 server error page when debug is false. I never get the "Backpack" error page, unless I throw an abort() or similar in a controller.

Can you reproduce it in a clean install ?

Cheers

amenk commented 1 month ago

I did not yet find the time to setup a clean install...

In the screenshot with debug on you can see that the exception message contains sensitive SQL info, somit should never be printed out if debug is false

the 500 server error page when debug is false

I think that's the problem..what server are you using? And which browser? Something seems to hide the template output on the 500er?

pxpm commented 1 month ago

Like you said, it must be server dependent, and I wasn't able to reproduce the issue. In any case, ensuring app.debug is true before outputting the error messages is a good security check.

Thanks for the suggestion, I've already merged the fix and will tag a new version later today 👍

amenk commented 1 month ago

thank you