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

Already on GitHub? Sign in to your account

Potential arbitrary read through uploaded vcard #4817

Closed
rcubetrac opened this Issue May 6, 2015 · 7 comments

Comments

Projects
None yet
1 participant

Reported by nrogers on 6 May 2015 16:37 UTC as Trac ticket #1490379

There is a potential for an arbitrary read from an authenticated user who uploads a contact (vCard) with a specially crafted POST. Though I couldn't get the bug to trigger myself, this code still looks dangerous:
\program\steps\addressbook\photo.inc @ line 67-96

If the $data value can be set to NULL (through vCard) and the _alt parameter is a valid file on the server an arbitrary read could occur:

if (!$data && ($alt_img = rcube_utils::get_input_value('_alt', rcube_utils::INPUT_GPC)) && is_file($alt_img)) {
$data = file_get_contents($alt_img);
}

This is returned via echo() a few lines down:
if ($data) {
header('Content-Type: ' . rcube_mime::image_content_type($data));
echo $data;
}

Though I was unable to find a way to have data=NULL the concerning part is the _alt field. There doesn't seem to be a corresponding feature for this in the GUI. Might be wise to remove if the functionality isn't used.

Attached is an example POST command to trigger this potential bug by supplying the "_alt" param in the POST. User must be authenticated

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

Comment by @alecpl on 7 May 2015 06:25 UTC

The _alt argument is indeed not used since 681ba6f so we can safely remove that code.

Milestone changed by @alecpl on 7 May 2015 06:25 UTC

later => 1.1.2

Comment by @alecpl on 7 May 2015 06:38 UTC

I was able to read any file on disk (the apache has access to, e.g. config/config.inc.php) using GET request. Raising priority. Note that we'd need a separate fix for 1.0 (where the _alt attribute is still used by program/steps/mail/show.inc).

Severity changed by @alecpl on 7 May 2015 06:38 UTC

minor => major

Comment by @alecpl on 7 May 2015 06:46 UTC

Fixed in e84fafc. Fix for 1.0 is pending.

Comment by @alecpl on 7 May 2015 07:03 UTC

Looks like 1.0 is also not really using _alt argument. Fixed in 6ccd4c5.

Status changed by @alecpl on 7 May 2015 07:03 UTC

new => closed

@rcubetrac rcubetrac closed this May 7, 2015

@rcubetrac rcubetrac added this to the 1.1.2 milestone Mar 20, 2016

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