Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Light infodisclosure issue #4816

Closed
rcubetrac opened this issue May 5, 2015 · 13 comments
Closed

Light infodisclosure issue #4816

rcubetrac opened this issue May 5, 2015 · 13 comments

Comments

@rcubetrac
Copy link

Reported by adprotas on 5 May 2015 19:46 UTC as Trac ticket #1490378

The logs directory is not protected from browsing. Most log entries are not bad, but one became evident on my host that was pretty nasty.

It looked like the following:

[04:03:11 -0400](25-Apr-2015): <ijpv9kqo> DB Error: [1062] Duplicate entry 'ijpv9kqofvpksxxxxxxxxxxxx' for key 'PRIMARY' (SQL Query: INSERT INTO `session` (`sess_id`, `vars`, `ip`, `created`, `changed`) VALUES ('ijpv9kqofvpksxxxxxxxx', 'xxxxxxxxxxxxxxxxxxxxxxx=', '108.61.90.131', now(), now())) in /var/www/html/roundcubemail-1.1.1/program/lib/Roundcube/rcube_db.php on line 543 (POST /roundcubemail-1.1.1/?_task=mail&_action=refresh?_task=&_action=)

I obfuscated the sensitive fields, but this would be enough for a non-credential user to view the file (via the webroot/logs/errors file), and then replace their own cookies with the entry from above to log in as a user that was listed there.

This seems to be a very rare occurrence, but considering that other SQL/other actions might report other sensitive data into this file, it might be worth automatically protecting this directory with an .htaccess file, or prepending a php tag to avoid overt reading by any unauthenticated user.

Migrated-From: http://trac.roundcube.net/ticket/1490378

@rcubetrac
Copy link
Author

Comment by @thomascube on 6 May 2015 06:29 UTC

The Roundcube package already comes with a .htaccess file to protect certain locations such as logs and temp directories. We also hint to protect these locations in our [wiki:Howto_Install] page. A specific .htaccess file in the logs folder could indeed be added to protect it in case the rewrite rules do not work as intended.

@rcubetrac
Copy link
Author

Milestone changed by @thomascube on 6 May 2015 06:47 UTC

later => 1.1.2

@rcubetrac
Copy link
Author

Comment by adprotas on 6 May 2015 07:00 UTC

Interesting. The .htaccess didn't seem to apply to my installation on Ubuntu 14.04.1. I'll try to investigate to see why this wasn't the case for our installation and report back.

@rcubetrac
Copy link
Author

Comment by adprotas on 6 May 2015 07:12 UTC

Confirmed that my Ubuntu installation was not picking up the .htaccess file. Fixed and you can close this issue. Sorry.

@rcubetrac
Copy link
Author

Comment by @thomascube on 6 May 2015 08:02 UTC

If it didn't apply to your default installation and you didn't notice the instructions, we should probably try to improve this on our side or at least raise the awareness of protecting certain resources. Therefore keeping the ticket open.

@rcubetrac
Copy link
Author

Comment by @alecpl on 6 May 2015 08:27 UTC

@adprotas: .htaccess didn't apply at all or only the rewrite rules? Because if .htaccess didn't work at all, putting a separate file in the temp directory wouldn't help here anyway.

I suppose we should just add SECURING section to INSTALL file (or extend the INSTALLATION section).

@rcubetrac
Copy link
Author

Comment by @thomascube on 6 May 2015 09:29 UTC

Replying to alec:

@adprotas: .htaccess didn't apply at all or only the rewrite rules? Because if .htaccess didn't work at all, putting a separate file in the temp directory wouldn't help here anyway.

Those separate files don't need to do rewrite rules but just a generic Deny from all.

I suppose we should just add SECURING section to INSTALL file (or extend the INSTALLATION section).

I'm just about to write this section.

@rcubetrac
Copy link
Author

Status changed by @thomascube on 6 May 2015 09:29 UTC

new => assigned

@rcubetrac
Copy link
Author

Owner changed by @thomascube on 6 May 2015 09:29 UTC

=> thomasb

@rcubetrac
Copy link
Author

Comment by @alecpl on 6 May 2015 09:44 UTC

Replying to thomasb:

Replying to alec:

@adprotas: .htaccess didn't apply at all or only the rewrite rules? Because if .htaccess didn't work at all, putting a separate file in the temp directory wouldn't help here anyway.
Those separate files don't need to do rewrite rules but just a generic Deny from all.

Sure, but the main .htaccess file does it using rewrites. That's why my question.

@rcubetrac
Copy link
Author

Comment by @thomascube on 7 May 2015 09:03 UTC

.htaccess files plus instructions added in commit 012555c

@rcubetrac
Copy link
Author

Status changed by @thomascube on 7 May 2015 09:03 UTC

assigned => closed

@rcubetrac
Copy link
Author

Comment by @thomascube on 7 May 2015 09:33 UTC

Backported to release-1.1 in 16640c7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants