RemoteUser-supplied local file paths

Mark Doliner mark at kingant.net
Tue Jan 22 02:05:00 EST 2013


On Thu, Jan 10, 2013 at 1:25 AM, Mark Doliner <mark at kingant.net> wrote:
> In 2.x.y these files call purple_imgstore_add_with_id() and set the
> filename to an unescaped value read from the network:
> - libpurple/protocols/gg/gg.c:1402
> - libpurple/protocols/oscar/odc.c:359
> - libpurple/protocols/sametime/sametime.c:2785
> The file is then written to disk in purple_buddy_icon_data_cache()
> using the unescaped filename.

I was wrong about this!  Those three places DO set an unescaped value
as the filename for a PurpleStoredImage, but those images are not
automatically written to disk.  The PurpleStoredImages are handed off
to gtkimhtml via an <img src="4"> tag.  I think the only places these
filenames are used is the default filename when opening a file
browsing dialog when the user right-clicks on an image and chooses
Save As.  And so I don't think we need to do anything about this case.

purple_buddy_icon_data_cache() is used for something different.  It
does write files to disk, but the filenames are always hashes.

> Possible fixes:
> 1. Update those three prpls to escape the filename using
> purple_escape_filename() before calling purple_imgstore_add_with_id().

This is still a good idea.  I'll probably do this in main.

> 2. Update purple_imgstore_add_with_id() to escape the filename in the img.

I think this isn't a valid option.  The filenames are allowed to be
either a basename or a full path, so we don't know whether path
separation characters are valid (because the filename is a full path),
or invalid (because some remote user snuck one in and it didn't get
escaped by the PRPL).

> 3. Update purple_buddy_icon_data_cache() to escape the filename before
> writing to disk.

This is irrelevant.  The buddy icon caching code always uses checksums
for filenames.


More information about the security mailing list