Fix creation and import Exploit

[xSavior-of-God]

Through this fix you can avoid deleting of important folders.

[nicegamer7]

Thanks for the PR!

Here are my thoughts.

[xSavior-of-God]

If there are no other corrections, I will modify the code and repeat the pull request if necessary.

[nicegamer7]

I'm not actually authorized to accept PRs, so I'd wait until the main dev gives his thoughts until you redo anything.

[dumptruckman]

If there are no other corrections, I will modify the code and repeat the pull request if necessary.

Sorry for the delay in checking this out. I made some comments so yeah, need a couple changes but otherwise this is great.

[benwoo1110]

Will be integrating this in ACF Command rework. Not sure if we want to merge it before that, like v4.3?

[benwoo1110]

@xSavior-of-God Are you still interested in proceeding on this PR?

[xSavior-of-God]

Yes, i need this fix!

[benwoo1110]

I think I recently added something similar, see https://github.com/Multiverse/Multiverse-Core/blob/main/src/main/java/com/onarandombox/MultiverseCore/utils/WorldNameChecker.java

I suggest you take a look at it and see if it has fixed the exploits you are concern with. I haven't use WorldNameChecker in the create command class, but it's probably easy to implement.

Another thing is I heard of people having sub-folders dir for worlds e.g. hub/worldname, since / is a valid char world worldname. Seems like this pr will prevent this behaviour?

[xSavior-of-God]

1 year ago I said how to fix/remedy the problem... and to date it has not been fixed...

I also mentioned how to replicate it, which to date may have caused problems for some servers owners... with the hope that it will be fixed as soon as possible, but this was not...

So now I tell you how things are...

Did you know that if you want you can create a world inside folders like /sys/, /var/, /usr/, /root/, /home/, etc...?
I don't know how safe it is to use Multiverse on a productive server... ok it's not recommended to start your server with root user, it's recommended to limit the permissions to the user who start the server.
Everyone obviously manages as they see fit, but a small and simple control or just a limitation via config (such as: "Hey by activating this you can risk hack attacks, such as deletion of important folders...") it could already be convenient, like I just enable it when I need, and I disable it when I don't need it anymore. (also via API if you use mv)

In any case, for example, it would be enough to know the path of the server folder to completely destroy it (or just use "./" that refer to the root of your server) by creating a world inside it and then deleting it... honestly, I told you how to do it and how to fix it, I think, it should be good a simple check of the name that removes the characters like '/', '\' and '.'...

---

However the problem still persists!

How to replicate delete plugins folder?

Simple just type this 2 commands:

/mv create ./plugins/test.dat/dio/ normal
/mv import ./plugins/ normal

NB: Do not use contains in reverse!

Multiverse-Core/src/main/java/com/onarandombox/MultiverseCore/utils/WorldNameChecker.java

Lines 39 to 54 in 28cee29

	/**
	* Checks the current validity status of a world name.
	*
	* @param worldName The world name to check on.
	* @return The resulting name status.
	*/
	@NotNull
	public static NameStatus checkName(@Nullable String worldName) {
	if (BLACKLIST_NAMES.contains(worldName)) {
	return NameStatus.BLACKLISTED;
	}
	if (worldName == null || !WORLD_NAME_PATTERN.matcher(worldName).matches()) {
	return NameStatus.INVALID_CHARS;
	}
	return NameStatus.VALID;
	}

do it like this

for(String check : BLACKLIST_NAMES) 
  if (worldName.contains(check))
    return NameStatus.BLACKLISTED;

---

I hope you have understood the seriousness of the problem and that I have not offended you (in case sorry), but above all I hope that the problem will be solved as soon as possible!

[benwoo1110]

ideally, we should check based on directory instead of just removing all /, possibly:

1. Ensure target world folder is within the world container path, not /home /root as you said.
2. and also it is not sub directory path of certain blacklisted folder, e.g. plugins/ logs/

so that we control, but not straight up deny worlds in sub directory, to prevent breaking changes while fixing this exploit.

[benwoo1110]

I will merge it as it is, and look into configurable options, as well as reducing false positives as I mentioned in the near future.

we merge it first

[xSavior-of-God]

Why do you have to get too complicated?
You can simply make an option in the config that makes it possible to use the '/', '\' and the '.', doing this avoids the biggest problem, then check if it is an important folder like: logs, plugins, libs, etc... (also this last if configurable would be the top)

[benwoo1110]

it's not... too complicated, I'm just providing ideas into how we can improve the world name checking system. Yes - things at mv are slow at times, but we do our best with the limited free time we have.