On 4/19/12, suzuki toshiya <[email protected]> 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?". > Umm.
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. wbr. > Ihar `Philips` Filipau wrote: >> On 4/19/12, Albert Astals Cid <[email protected]> wrote: >>> You can do >>> pdftohtml -c -dev 'jpeg /dev/null;cat /etc/passwd;#' >>> /path/to/some/pdf/fil >>> 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 >>> it's >>> 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 >>> system() >>> 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 >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/poppler > > -- 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) _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
