ownCloud showing MySQL error in the ownCloud Desktop client

#1

Hello everyone,

I set this thread in the server section since I believe it’s a server problem even though the behavior is seen in the client.
Anyway, I’m running an ownCloud 10.2.1 instance, and when I have upload errors with the Desktop Client (2.6.0) on Windows 10 I can see the MySQL requests causing the error:

Example:

15/11/2019 15:26:08, xxx/yyyy/zzzz.pptx
ownCloud4,Error transferring https://xxx.com/remote.php/dav/uploads/xxx/389833367/0000000010000000 - server replied: (An exception occurred while executing 'SELECT `fileid`, `storage`, `path`, `parent`, `name`, `mimetype`, `mimepart`, `size`, `mtime`, `storage_mtime`, `encrypted`, `etag`, `permissions`, `checksum` FROM `oc_filecache` WHERE `storage` = ? AND `path_hash` = ?' with params [7, "a25301b1d1ebdf0e451ed8b7e741a315a"]: SQLSTATE[HY000]: General error: 2006 MySQL server has gone away)

Here’s my log configuration for the config.php

  'loglevel' => 2,
  'log_type' => 'syslog',

Is it possible to hide the MySQL errors in the Desktop client ? I find this “dangerous” since it could give the structure of the database to any logged users. Also it’s not really “nice” to see that in a Desktop application, the error should be more generic and be show only when ticking a debug option for example.

Thanks.

#2

Please open an issue at https://github.com/owncloud/core/issues/
I’ve seen similar exposure also in the web gui. At the client side (web gui or desktop client) we can hardly guess which messages should be shown to the user, and which not.

I’d say it is the server’s responsibility to generate meaningful messages, but avoiding potential exposure. A good solution would be to include a logfile token in the error message, and say, for further details consult the server logs looking for $TOKEN.

2 Likes
#3

Hello,
thanks for your answer !
This solution would suit me, as long as the messages don’t expose too much information on the structure, I’m fine with it. Also, maybe if you’re an admin, you could have these very detailed informations and not for normal users.
I’ll open an issue and link it here to follow-up, I thought it might be some misconfiguration on my side.

#4

Hey,

while i think such detailed information shouldn’t be shown to the client i don’t think that this specific message is exposing currently unknown information about the database. AFAIK the database structure of ownCloud is known / publicly available so i’m not sure if there is anything “dangerous” exposed here.

1 Like
#5

Hey tom42,

well you’re correct, it’s true that the database structure is available to anyone. Even with a table prefix an attacker could guess the prefix with a correct SQL injection.

I still open an issue since IMHO, the SQL shouldn’t be seen to the client.

1 Like
#6

Re,

there goes the issue on github.

1 Like
#7

Hey,

yes i think this is absolutely true as also mentioned previously: :slightly_smiling_face:

I just wanted to add some notes on the “dangerous” part which might be not that dangerous. :slightly_smiling_face:

1 Like
#8

Hey,

and you were right to clarify, I didn’t consider it that dangerous since I wrote “dangerous” at first, even though I don’t like the database structure to be shown but security through obscurity is no security. Changing the table prefix might prevents from some very specific attacks but wouldn’t secure the database against someone experienced.

1 Like
#9

Are you sure those are “upload errors” for me this means that the database is not reachable.
Otherwise agree, we like to catch such statements and eliminate them as sometimes they can reveal unnessary information - even if not in this particular case. They are also very confusing to end users.
So can you confirm what the error actually was - really a non-reachable database or something else?

2 Likes
#10

Hey @hodyroff

thanks for your concern :slight_smile:
Well my customer gave me the error from its ownCloud Desktop Client, so to me they are upload errors or at least related to the client <-> server relationship. The MySQL server gone away can happen when the network gets slow (wait_timeout is reached), too many connections, max_allowed_packet too small etc … So after thorough investigations, I fixed all the issues and don’t have feedback about this error but it could occur again for the reasons mentioned. Btw I guess it could be easily reproduced by lowering drastically the MySQL configuration parameters (max_connection, packet size, timeouts, …) and for example throttling your connection when you upload stuff.
Thanks.

1 Like
#11

From my point of view, any unhandled exception needs to be shown somehow so either the user or the admin can take an appropiate action. If you didn’t receive that error, you could have fixed because you wouldn’t know you have a problem with the DB.

I agree that the exception is ugly and it needs to be handled properly, but if we just hide the exception it will become impossible not only to fix the problem, but also to know what’s happening. It could have been easy to mistake this error with a “my files don’t upload” or “random failure uploading files” which makes both you and developers to waste a lot of time

2 Likes
#12

@jvillafanez

I agree with your opinion, errors should be displayed but in a way that the user doesn’t get panicked.
How about a simple error code in the Desktop Client with a link to a database of possible causes for every error code ? This way it wouldn’t be overwhelming to have a big SQL query, you’d just follow the link and have something like “Check your MySQL configuration it could be packet size, max connections, etc …”. And everyone could contribute to improve this database by giving advices, fixes.

1 Like
#13

IMHO, this looks like a misconfiguration from the DB Server. It could be that the sync-client has too many files and is taking too long to provide all the files structure to the client.

It would be good if you could increment the connection time from the DB and see if the behaivior works better.

#14

Hello,

thanks for not reading the purpose of this thread --’
Again the topic is not about the MySQL error, it’s wider, I know how to fix it it’s been fixed yet.
It’s about not showing too much information to overwhelm “basic” users.

2 Likes
#15

I read the thread, and as I SAID “my opinion” is the message will facilitate the administrator to find the solution.

As @jnweiger said before the client could have in the future a token, but so far is useful output that a basic user could copy and paste to the administrator.
And I agree with @tom42 too, there is nothing dangerous exposed and basic user can’t do anything to harm.

#16

Hey,

it seems ownCloud itself is already doing this when facing an internal server error like seen in the screenshot below:

AFAIK the “Request ID” is the ID which is matching the related log entry in the owncloud.log. I think exactly the same could be done by the sync client as well.

1 Like
#17

Actually, in the case of the database not reachable, we show the same ugly error in the web as in the client, no difference. We will look into it …

2 Likes
#18

That would be really great ! FYI the “issue” is here.

2 Likes
#19

Hey,

could it be possible that this is already handled in the following issue?

#20

Probably yes, but due to other reasons. It might fix the problem because the DB is down and somehow we need the DB in order to log “normally” (this is probably to get and log the display name of the user). Since we couldn’t log “normally”, we’ll have to log the problem “abnormally”.

Note that other DB errors that could happen while the DB is still up, might still reach the client.

1 Like