The work that had already been done by Richard intends to remove any uploaded file from the local disc, if it has been determined that the file is not a JAR archive. This in itself should sufficiently resolve this ticket.

I've provided further improvements in https://github.com/igniterealtime/Openfire/pull/1440

This PR adds three new checks that are applied to to-be-installed plugin files:

1. A check if the browser-reported 'content-type' header matches that of a JAR file. This check is disabled by default, as I suspect it'd be trivial for a malicious attacker to circumvent this check.

2. A validation of the first few bytes of the uploaded data (the magic bytes). Enabled by default. If these do not match a recognized JAR/ZIP pattern, the uploaded file is not even being written do disc (further reducing any attack vector that involves sending malicious files).

3. Verification of the presence of a 'plugin.xml' entry in the JAR file. Enabled by default. This primarily is a UX improvement, as the current code happily accepts _any_ jar file, without giving a strong visual indicator that the plugin installation failed.

Not sure what the threat here is once the other aspects are addressed.

The files are still written with a ".part" extension and then unlinked if they fail parsing in 2017-09-28 nightly (to hand)

[pid   919] lstat("/usr/share/openfire/plugins/fred fred.jar.part", 0x7f3c260cb140) = -1 ENOENT (No such file or directory)

[pid   919] open("/usr/share/openfire/plugins/fred fred.jar.part", O_WRONLY|O_CREAT|O_EXCL, 0666) = 130

[pid   919] stat("/usr/share/openfire/plugins/fred fred.jar.part", {st_mode=S_IFREG|0644, st_size=6, ...}) = 0

[pid   919] open("/usr/share/openfire/plugins/fred fred.jar.part", O_RDONLY) = 130

[pid   919] lstat("/usr/share/openfire/plugins/fred fred.jar.part", {st_mode=S_IFREG|0644, st_size=6, ...}) = 0

[pid   919] unlink("/usr/share/openfire/plugins/fred fred.jar.part") = 0

I did note in reviewing the BurpSuite test box that lots of corrupted file names were present in this folder during Burp Suite testing, but I presume that was from fiddling with the admin interface in manipulating plugin names as admin, rather than via upload.

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 %2fetc%2fpasswd

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 ${464*355}

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 #{464*355}

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 %{6942*2760}

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 8067*924

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 ${9984*7525}

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 a'a\'b\"c>?>%}}%%>c<[[?${{%}}cake\

drwxr-x--- 3 openfire openfire  4096 Sep 28 14:11 admin

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 ${applicationScope}

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 #{applicationScope}

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 %E5%98%8A%E5%98%8DX-Injection:%20test

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 empty

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 none

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 null

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 od3x4\z`z'z\"${{%{{\

rw-rr- 1 openfire openfire 30412 Oct 11 17:43 out.zip

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 out.zip|zq=%3c%61%60%27%22%24%7b%7b%5c

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 out.zip&zq=%3c%61%60%27%22%24%7b%7b%5c

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 out.zip$zq=%3c%61%60%27%22%24%7b%7b%5c&zq%3d

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 out.zip&zq=x%3c%61%60%27%22%24%7b%7b%5c

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 passwd

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 passwd\").getInputStream()),\"UTF-8\")).useDelimiter(\"\\A\").next()

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 s5v9oyf0

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 sleep 0

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 `sleep 11`

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 $(sleep 11)

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 sleep 11

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 sleep 11; }

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 sprimtf

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 sprintf

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 sprintg

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 ttx04b88b16`z'z\"${{%{{\

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 ujcc''hnfh

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 undefined

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 web.xml

rw-rr- 1 openfire openfire 30412 Oct 11 17:05 web.xml;x=

Should this have been closed? Rich's PR was merged. Or did we decide that it only limited to zip files, basically?

Not retested at 4.1.beta as the issue noted on means we should fix this ASAP.

Basically, allows any file type to be uploaded to the plugins directory on the server. Should be restricted to JAR files only, if that's the only thing supported.