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