GraphicsMagick and ImageMagick popen() shell vulnerability via filename

Re:GraphicsMagick and ImageMagick popen() shell vulnerabilit... by yxhuvud on Monday May 30, 2016 @03:33AM (#11800087)

This is not the first time popen with | has led to security issues. At the same time it doesn't have any legitimate use cases that I can think of. Is there any good reason to not patch popen and remove the support for |s instead?

[ Parent | on HN ]

Re:GraphicsMagick and ImageMagick popen() shell vulnerabilit... by MereInterest on Monday May 30, 2016 @07:18AM (#11800746)

It can be useful if you want to offload some of the processing to other processes.

    // Open a gzipped file, display progress bar
    snprintf(buf, sizeof(buf), "pv %s | zcat", filename);
    FILE* f = popen(buf, "r");

    // Add an additional processing step when reading file
    FILE* f = NULL;
    if(do_extra_step) {
        snprintf(buf, sizeof(buf), "zcat %s | extra_step", filename);
        f = popen(buf, "r");
    } else {
        snprintf(buf, sizeof(buf), "zcat %s", filename);
        f = popen(buf, "r");
    }

The first example could be replicated by using zlib and reproducing pv's progress bar in-app. More work, but nothing especially tricky about it.

For the second example, it would be more difficult, for two reasons. First, popen() can open either a read or a write pipe, but not both. In order to write to extra_step's stdin, then read from stdout, you'll need to switch over to fork/exec. Second, you'll have to handle all the buffering between zcat and extra_step yourself, rather than getting it for free.

[ Parent | on HN ]