View Issue Details

IDProjectCategoryView StatusLast Update
0011395mantisbtsqlpublic2010-04-23 14:30
Reporteratrol Assigned Tojreese  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionno change required 
Product Version1.2.0rc2 
Target Version1.2.1 
Summary0011395: show_queries_list gives warnings instead of displaying queries
Description

setting show_queries_list to value 1 with Manage -> Manage configurations leads to warnings
SYSTEM WARNING: Cannot use a scalar value as an array

setting to value ON in config_inc.php works fine

Additional Information

nightly build from 14.01.2010

TagsNo tags attached.

Activities

dhx

dhx

2010-01-15 09:05

reporter   ~0024159

This setting should be "global" in that it can only be changed from config_inc.php (and not from within the database configuration).

I've patched 1.2.x and 1.3.x to mark the showqueries options as global, preventing them from being looked up (and stored) in the database configuration.

Thanks for the report.

atrol

atrol

2010-01-15 09:32

developer   ~0024160

I feared, that this will be your solution ;-)

In 1.1.8 I was able to enable the query list feature for a single user.
For example the administrator is able to examine queries, while the other users can work without displaying the lists.

Is there a way to achieve this in config_inc.php?

atrol

atrol

2010-01-15 09:33

developer   ~0024161

Last edited: 2010-01-15 09:34

Please also have a look at http://www.mantisbt.org/bugs/view.php?id=11038 where I had another discussion concerning "what should be done only in config_inc.php, what in database"

dhx

dhx

2010-01-17 19:12

reporter   ~0024188

One of the concerns we have (and this is one of the key reasons why $g_global_settings exists) is that someone with access to the database - or access to a MantisBT administrator account - shouldn't be able to reconfigure MantisBT to access or execute arbitrary files on the hard disk. Ideally we shouldn't be trusting the data we pull from the database (because of issues like XSS injection via SQL injection or a cracked database password). We're not at that stage yet, of course.

I do agree that allowing most configuration options to be stored in the database is a good idea. However, paths, specification of executable binaries (for graphviz, etc), URLs, page names, etc should generally be banned from being placed in the database IMO. By "banned", I mean that a user could easily reenable them if they understand the risks, but the default state would be to not allow those settings in the database.

By storing paths and other sensitive settings in the database, we effectively open up the door to allowing malicious users with database access (or administrator access to set configuration values in the database) to:
1) Change settings pointing to binaries that should be executed (ie. they might change it so a remote shell is opened instead of the graphviz binary)
2) Change paths and filenames so that hidden files on the disk are printed or displayed by MantisBT
3) Change URLs so that users are redirected to malicious websites (or are told by their browsers to retrieve externally hosted malicious files)

What we really need is a distinction between "protected", "per-project", "per-user" and "global" options.

Protected: never store in the database
Per-project: configurable on a per-project basis, but not per-user
Per-user: configurable on a per-user basis, but not per-project
Global: not configurable on a per-project and/or per-user basis

We also then need to treat configuration from the database as being potentially malicious in nature. It may contain bad HTML/javascript (thus needs to be sanitised before being outputted).

atrol

atrol

2010-01-18 05:28

developer   ~0024191

I agree with most of what you wrote.
Please check whether the setting we are talking about (show_query_list) belongs to one of the categories that you see as critical. I think not.

With your current changes maybe it is more unsecure than before, because I have to turn on queries list for all users.
To be honest, I don't know whether seeing the queries can help to attack an installation.
At first sight, I see not much more than I can see in the sourcecode. Maybe to see the real name of the database tables if not starting with mantis_ can be used for an attack.

IMO developing the things "What we really need ..." should not be the reason that 1.2.0 final has further delay.
Be aware that 1.2.0a1 was released 2008-04-18
But I agree, this is "What we really need" + Per-project/user (configurable on a per-project/user basis)is needed (for example this is already implemented in the column management where every user can have it's own view columns per project)

There are two tasks:
1) This issue as a bug which would be nice to be removed in 1.2.0 ("Cannot use a scalar value as an array")
2) A new issue with "What we really need ..." as a feature request for 1.3

If 1) is a lot of effort and/or may lead to further problems, I would prefer if you could concentrate your time to finish 1.2.0 final or a RC3

cor3huis

cor3huis

2010-01-22 11:15

reporter   ~0024224

Well put Atrol. BTW I also do not like feature creep, but IMHO The rest of this discussion maybe better to be continued in the forum.

atrol

atrol

2010-01-22 15:05

developer   ~0024229

Why move to forum?
We are talking about a bug and a feature request.
I am happy that dhx as one of the developers takes the time to reflect on this.
He can decide whether he wants to clone the issue to track both, the bug and the request and maybe close this one for 1.2 .
There is information in the tracker, which will be hard to keep together when moving to the forum and moving back to the tracker, maybe months later.

Be aware, that most of the MantisBT developers do hardly contribute to the forum.
I think they would not have much time to develop anything if they would spent much time watching the forum.
BTW, I have not much time to develop anything, because I am one of the moderators of the forum.
This should be discussed in the forum ;-)

dhx

dhx

2010-02-12 08:17

reporter   ~0024380

atrol: sorry for the delayed response, I do plan on seeing if I can make this option work on a per-user basis again. And thanks for your help in the forums and on this bug tracker, it's a huge help to the developers.

jreese

jreese

2010-04-19 15:43

reporter   ~0025157

There should be no reason to set this in the database. By default, setting show_queries_list obeys the access level specified in show_queries_threshold, which defaults to ADMINISTRATOR. Assuming you trust anyone you've given ADMIN access to, this should already be doing exactly what you are trying to make it do...

Related Changesets

MantisBT: master-1.2.x f3d5815f

2010-01-15 09:01

dhx


Details Diff
Fix 0011395: show_queries_list should be a global option Affected Issues
0011395
mod - config_defaults_inc.php Diff File

MantisBT: master e22eeacf

2010-01-15 09:03

dhx


Details Diff
Fix 0011395: show_queries_list should be a global option Affected Issues
0011395
mod - config_defaults_inc.php Diff File