[poppler] pdftohtml lets you run random shell commands
Ihar `Philips` Filipau
thephilips at gmail.com
Thu Apr 19 02:42:24 PDT 2012
On 4/19/12, suzuki toshiya <mpsuzuki at hiroshima-u.ac.jp> wrote:
> Ahh, indeed, I think you say that "pdftohtml is dangerous as find or xargs
> commands that can invoke any other command, but these are security issue?".
Something like this.
"Injecting stuff is bad" is clearly speak of security types. So the
follow up logical question: do we have any security concerns here?
Answer is "no." Thus injecting stuff is not security problem, but
rather usability problem of passing complex arguments. Usability
question: does one really need to put anything special into the device
string? Answer is again "no," - it is a plain string where "gs"
doesn't even expect spaces.
Thus to me, in the end, such toying with -dev parameter is nothing
major and is of "doctor it hurts! don't do it then" type of problem.
I doubt my voice should count (because I work with a rather limited
subset of PDFs) but I too do not mind removal of the stuff since I
never used it. But as a software developer, neither I see any risk of
it being left as it is.
> Ihar `Philips` Filipau wrote:
>> On 4/19/12, Albert Astals Cid <aacid at kde.org> wrote:
>>> You can do
>>> pdftohtml -c -dev 'jpeg /dev/null;cat /etc/passwd;#'
>>> and voila, you'll get your /etc/passwd printed on screen
>>> Definitely not nice.
>>> This is because we are using plain system() to run the gs command and
>>> easy to inject stuff there
>> My 0.02€
>> So what? User already can print /etc/passwd.
>> The problem of system() call is only relevant when the command is
>> installed suid-root(*). And pretty much all systems install only
>> required minimum of commands as suid-root. ((*) Or user convinces
>> admin to run something as root in his own terminal - but you can't
>> really do anything against idiot admins.)
>>> The real solution is moving to a fork+exec solution (path attached).
>> You use execvp() - that doesn't improve anything: the 'p' letter in
>> execvp() stands for "path resolution", meaning that user can still add
>> its own wrapper for "gs" command, adjust the $PATH and circumvent
>> whatever you intended to prevent with the patch.
>> IOW, the exec*p*() functions are as insecure as the system() - unless
>> of course you use absolute path for "gs" (what I gather would cause
>> troubles for portability).
>> Simpler /fix/ would be to make the programs not runnable by root -
>> `geteuid() != 0`. That would also cover the case of idiot admins. :)
>> Otherwise, in the patch, if one would replace the kinky va_list stuff
>> with a GooList() of `char *` (iow, pack the command line onto the list
>> (and add an accessor for the GooList::data)) the change would easily
>> come off as a clean up. ;)
>>> The problem with that is that we loose support for platforms with
>>> and without fork+exec (Windows).
>> The problem doesn't exist on Windows, since it doesn't have anything
>> like suid. It's either user or Administrator(**). And if user can run
>> a program as an administrator, then all bets are off. (The same case
>> as an idiot *nix admin.)
>> (**) Win Vista/7 have something similar, but UAC would bark at it. So
>> it doesn't change the parity.
>> poppler mailing list
>> poppler at lists.freedesktop.org
Don't walk behind me, I may not lead.
Don't walk in front of me, I may not follow.
Just walk beside me and be my friend.
-- Albert Camus (attributed to)
More information about the poppler