Possible CommandInjection in: \source\ij\plugin\BrowserLauncher.java

[rebujacker]

Hello imageJ staff/community,

I would like to report some security issue that will affect to ImageJ software running on OSX systems.

I think that in this piece of code we could inject commands:

BrowserLauncher.java ==> openURL(String url)

    public static void openURL(String url) throws IOException {
        String errorMessage = "";
        if (IJ.isMacOSX()) {
            if (IJ.isJava16())
                IJ.runMacro("exec('open', '"+url+"')");
            else {
...

I think it will be possible to inject code inside runMacro() in the following way:

BrowserLauncher b = new BrowserLauncher();
String imageFilePath = "http://127.0.0.1/test.jpg');exec('open -a /Applications/Calculator.app');exec('\"";
b.openURL(imageFilePath);

Sorry if it is a known issue, or there are technical errors,

Alvaro Folgado

[ctrueden]

@AlvFolgado Thanks for the report, but I am confused: could you not also simply run the code using the macro language's eval function? Or via any of ImageJ's supported script languages? Going through the BrowserLauncher seems awfully convoluted by comparison.

By the way: the ij.plugin.BrowserLauncher class lives here, not in this repository.

[rebujacker]

As far I have research, it looks like only by calling that function (BrowserLauncher.openURL) this attack could be possible. Of course this reduce their impact drastically (adding that only works in OSX systems). Normally almost no one is going to use this class/method to directly retrieve an image from an URL (we have theoretically safe function as "openImage()"), but maybe some developers will see an interest in use the app browser plugin to retrieve it in some point of development, and feeding it with user Input. On the other hand, it will be wise to use a safe function to avoid the exploit of it in the future.

Thank for the fast response, Alvaro

[ctrueden]

The following Groovy script, when run from ImageJ's Script Editor, launches the Calculator on macOS:

import ij.IJ
IJ.runMacro("exec('open -a /Applications/Calculator.app')")

This is much more direct than your injection.

I agree that the BrowserLauncher code is insecure—but I want to understand how this hack actually makes users any more vulnerable than they already are by running ImageJ in general.

[rebujacker]

Problem comes when we feed Strings without any kind of control to dangerous functions. Let's imagine the following piece of code in a spring framework controller using imageJ as a 3PP library for image processing:

@RequestMapping(value = "/retrieveimageURL", method=RequestMethod.POST)
  public void dangerousfunction(HttpServletRequest request,HttpServletResponse response) throws IOException {

              String url = request.getParameter("url");

              BrowserLauncher b = new BrowserLauncher();

              b.openURL(url);
  }
}

Developer is trusting that openURL in browserLauncher.java can only open a browser with an URL and not to do more,like run other OS commands (by breaking the String). And here for example there is a safe way to use IJ.runMacro() without possibles command Injections:

Opener.java ==>

/** Opens a single TIFF or DICOM contained in a ZIP archive,
        or a ZIPed collection of ".roi" files created by the ROI manager. */    
    public ImagePlus openZip(String path) {
        ImagePlus imp = null;
        try {
            ZipInputStream zis = new ZipInputStream(new FileInputStream(path));
            ZipEntry entry = zis.getNextEntry();
            if (entry==null) {
                zis.close();
                return null;
            }
            String name = entry.getName();
            if (name.endsWith(".roi")) {
                zis.close();
                if (!silentMode)
                    if (IJ.isMacro() && Interpreter.isBatchMode() && RoiManager.getInstance()==null)
                        IJ.log("Use roiManager(\"Open\", path) instead of open(path)\nto open ROI sets in batch mode macros.");
                    else
                        IJ.runMacro("roiManager(\"Open\", getArgument());", path);
                return null;
            }

Imagine that instead of being using:

IJ.runMacro("roiManager(\"Open\", getArgument());", path);

The code would be using this unsafe way (like in BrowserLauncher):

IJ.runMacro("roiManager(\"Open\", '"+path+"')");

This for example, will be a possible critical command Injection by opening ".roi" image files with special crafted filename on it.

Thanks,

Alvaro

[ctrueden]

OK, that is fair. Thanks for the explanation. I think it is highly unlikely that anyone would write a web service (which runs headless on the server) that invokes the BrowserLauncher (which open a URL in a UI), but... I guess it is conceivable that someone could use ImageJ in an embedded way somewhere and conceivably be bitten by this exploit.

Would you have time and interest to file a PR in https://github.com/imagej/imagej1 with a fix?

[rebujacker]

Done. Thanks for the fast response!

[ctrueden]

For reference, the PR is imagej/imagej1#35. Thanks, @AlvFolgado.