From f5656b2ae1dbec6b3f19509aa15aa3b3093c7b45 Mon Sep 17 00:00:00 2001 From: low012 Date: Sat, 17 Oct 2009 00:26:14 +0000 Subject: [PATCH] *) Made sure that only files with appropriate file endings are listed as skin or language files. *) Introduced protection against directory traversal attacks in configuration servlets for skin and language configuration. Files can only be deleted if they are contained in a list of files which has been read by the servlet first. Until now it was possible to delete any data on a system YaCy is running on and which can be deleted by the user who's account has been used to start YaCy. Most of the times a user of YaCy is also the owner of the machine the peer is running on, but this might not always be the case and not even the owner of the machine should be able to use YaCy as a replacement for "rm" or "del". git-svn-id: https://svn.berlios.de/svnroot/repos/yacy/trunk@6423 6c8d7289-2bf4-0310-a012-ef5d649a1542 --- htroot/Blacklist_p.java | 6 ++-- htroot/ConfigAppearance_p.html | 2 ++ htroot/ConfigAppearance_p.java | 37 ++++++++++++++++------- htroot/ConfigLanguage_p.html | 2 ++ htroot/ConfigLanguage_p.java | 55 +++++++++++++++++++++------------- 5 files changed, 69 insertions(+), 33 deletions(-) diff --git a/htroot/Blacklist_p.java b/htroot/Blacklist_p.java index c2f3ef461..015906e5f 100644 --- a/htroot/Blacklist_p.java +++ b/htroot/Blacklist_p.java @@ -166,9 +166,9 @@ public class Blacklist_p { return prop; } - final File BlackListFile = new File(listManager.listsPath, blacklistToUse); - if(!BlackListFile.delete()) { - Log.logWarning("Blacklist", "file "+ BlackListFile +" could not be deleted!"); + final File blackListFile = new File(listManager.listsPath, blacklistToUse); + if(!blackListFile.delete()) { + Log.logWarning("Blacklist", "file "+ blackListFile +" could not be deleted!"); } for (int blTypes=0; blTypes < supportedBlacklistTypes.length; blTypes++) { diff --git a/htroot/ConfigAppearance_p.html b/htroot/ConfigAppearance_p.html index c7d5c995d..5aac1235e 100644 --- a/htroot/ConfigAppearance_p.html +++ b/htroot/ConfigAppearance_p.html @@ -48,6 +48,8 @@
+

Make sure that you only download data from trustworthy sources. The new language file + might overwrite existing data if a file of the same name exists already.

diff --git a/htroot/ConfigAppearance_p.java b/htroot/ConfigAppearance_p.java index 0678a6f72..0a0680ac3 100644 --- a/htroot/ConfigAppearance_p.java +++ b/htroot/ConfigAppearance_p.java @@ -52,7 +52,9 @@ import java.util.Collections; public class ConfigAppearance_p { - public static serverObjects respond(final RequestHeader header, final serverObjects post, final serverSwitch env) { + private final static String SKIN_FILENAME_FILTER = "^.*\\.css$"; + + public static serverObjects respond(final RequestHeader header, final serverObjects post, final serverSwitch env) { final serverObjects prop = new serverObjects(); final Switchboard sb = (Switchboard) env; final String skinPath = new File(env.getRootPath(), env.getConfig("skinPath", "DATA/SKINS")).toString(); @@ -61,26 +63,42 @@ public class ConfigAppearance_p { prop.put("currentskin", ""); prop.put("status", "0"); // nothing - List skinFiles = listManager.getDirListing(skinPath); + List skinFiles = listManager.getDirListing(skinPath, SKIN_FILENAME_FILTER); if (skinFiles == null) { return prop; } if (post != null) { - if (post.containsKey("use_button") && post.get("skin") != null) { - // change skin - changeSkin(sb, skinPath, post.get("skin")); + String selectedSkin = post.get("skin"); + if (post.containsKey("use_button") && selectedSkin != null) { + /* Only change skin if filename is contained in list of filesnames + * read from the skin directory. This is very important to prevent + * directory traversal attacks! + */ + if (skinFiles.contains(selectedSkin)) { + changeSkin(sb, skinPath, selectedSkin); + } + } + if (post.containsKey("delete_button")) { - // delete skin - final File skinfile = new File(skinPath, post.get("skin")); - FileUtils.deletedelete(skinfile); + + /* Only delete file if filename is contained in list of filesname + * read from the skin directory. This is very important to prevent + * directory traversal attacks! + */ + if (skinFiles.contains(selectedSkin)) { + final File skinfile = new File(skinPath, selectedSkin); + FileUtils.deletedelete(skinfile); + } } if (post.containsKey("install_button")) { // load skin from URL final String url = post.get("url"); + final File skinFile = new File(skinPath, url.substring(url.lastIndexOf("/"), url.length())); + Iterator it; try { final DigestURI u = new DigestURI(url, null); @@ -93,7 +111,6 @@ public class ConfigAppearance_p { return prop; } try { - final File skinFile = new File(skinPath, url.substring(url.lastIndexOf("/"), url.length())); final BufferedWriter bw = new BufferedWriter(new PrintWriter(new FileWriter(skinFile))); while (it.hasNext()) { @@ -112,7 +129,7 @@ public class ConfigAppearance_p { } // reread skins - skinFiles = listManager.getDirListing(skinPath); + skinFiles = listManager.getDirListing(skinPath, SKIN_FILENAME_FILTER); Collections.sort(skinFiles); int count = 0; for (String skinFile : skinFiles) { diff --git a/htroot/ConfigLanguage_p.html b/htroot/ConfigLanguage_p.html index 4f2aa39c7..29192d109 100644 --- a/htroot/ConfigLanguage_p.html +++ b/htroot/ConfigLanguage_p.html @@ -51,6 +51,8 @@
+

Make sure that you only download data from trustworthy sources. The new language file + might overwrite existing data if a file of the same name exists already.

#(status)# diff --git a/htroot/ConfigLanguage_p.java b/htroot/ConfigLanguage_p.java index 5854c59df..e1fe8c331 100644 --- a/htroot/ConfigLanguage_p.java +++ b/htroot/ConfigLanguage_p.java @@ -54,8 +54,10 @@ import java.util.Collections; public class ConfigLanguage_p { + private final static String LANG_FILENAME_FILTER = "^.*\\.lng$"; + public static serverObjects respond(final RequestHeader header, final serverObjects post, final serverSwitch env) { - //listManager.switchboard = (plasmaSwitchboard) env; + final serverObjects prop = new serverObjects(); final String langPath = env.getConfigPath("locale.work", "DATA/LOCALE/locales").getAbsolutePath(); @@ -63,20 +65,35 @@ public class ConfigLanguage_p { //prop.put("currentlang", ""); //is done by Translationtemplate prop.put("status", "0");//nothing - List langFiles = listManager.getDirListing(langPath); + List langFiles = listManager.getDirListing(langPath, LANG_FILENAME_FILTER); if(langFiles == null){ return prop; } if (post != null){ + String selectedLanguage = post.get("language"); + //change language - if(post.containsKey("use_button") && post.get("language") != null){ - translator.changeLang(env, langPath, post.get("language")); + if(post.containsKey("use_button") && selectedLanguage != null){ + /* Only change language if filename is contained in list of filesnames + * read from the language directory. This is very important to prevent + * directory traversal attacks! + */ + if (langFiles.contains(selectedLanguage)) { + translator.changeLang(env, langPath, selectedLanguage); + } //delete language file }else if(post.containsKey("delete")){ - final File langfile= new File(langPath, post.get("language")); - FileUtils.deletedelete(langfile); + + /* Only delete file if filename is contained in list of filesnames + * read from the language directory. This is very important to prevent + * directory traversal attacks! + */ + if (langFiles.contains(selectedLanguage)) { + final File langfile= new File(langPath, selectedLanguage); + FileUtils.deletedelete(langfile); + } //load language file from URL } else if (post.containsKey("url")){ @@ -111,7 +128,7 @@ public class ConfigLanguage_p { } //reread language files - langFiles = listManager.getDirListing(langPath); + langFiles = listManager.getDirListing(langPath, LANG_FILENAME_FILTER); Collections.sort(langFiles); final HashMap langNames = translator.langMap(env); String langKey, langName; @@ -123,20 +140,18 @@ public class ConfigLanguage_p { int count = 0; for(String langFile : langFiles){ - if(langFile.endsWith(".lng")){ - //+1 because of the virtual entry "default" at top - langKey = langFile.substring(0, langFile.length() -4); - langName = langNames.get(langKey); - prop.put("langlist_" + (count + 1) + "_file", langFile); - prop.put("langlist_" + (count + 1) + "_name", ((langName == null) ? langKey : langName)); - if(env.getConfig("locale.language", "default").equals(langKey)) { - prop.put("langlist_" + (count + 1) + "_selected", "selected=\"selected\""); - prop.put("langlist_0_selected", " "); // reset Default - } else { - prop.put("langlist_" + (count + 1) + "_selected", " "); - } - count++; + //+1 because of the virtual entry "default" at top + langKey = langFile.substring(0, langFile.length() -4); + langName = langNames.get(langKey); + prop.put("langlist_" + (count + 1) + "_file", langFile); + prop.put("langlist_" + (count + 1) + "_name", ((langName == null) ? langKey : langName)); + if(env.getConfig("locale.language", "default").equals(langKey)) { + prop.put("langlist_" + (count + 1) + "_selected", "selected=\"selected\""); + prop.put("langlist_0_selected", " "); // reset Default + } else { + prop.put("langlist_" + (count + 1) + "_selected", " "); } + count++; } prop.put("langlist", (count + 1));