From bc2c1ac828d5d7df8e0ba940c32b4e8b3bca4b14 Mon Sep 17 00:00:00 2001 From: dev747368 <48332326+dev747368@users.noreply.github.com> Date: Tue, 30 Jul 2019 16:50:24 -0400 Subject: [PATCH] GT-3042 allow back, forward slashes in user name. --- .../main/java/ghidra/server/ServerAdmin.java | 5 +- .../main/java/ghidra/server/UserAdmin.java | 31 +--- .../main/java/ghidra/server/UserManager.java | 143 ++++++++++-------- .../java/utilities/util/FileUtilities.java | 5 +- 4 files changed, 89 insertions(+), 95 deletions(-) diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/ServerAdmin.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/ServerAdmin.java index 2913e84549..66928c7c96 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/ServerAdmin.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/ServerAdmin.java @@ -289,16 +289,17 @@ public class ServerAdmin implements GhidraLaunchable { System.exit(-1); } String sid = args[i]; - if (!NamingUtilities.isValidName(sid) || sid.indexOf(' ') >= 0) { + if (!UserManager.isValidUserName(sid)) { Msg.error(UserAdmin.class, "Invalid username/sid: " + sid); System.exit(-1); } } /** - * Validate username/sid + * Validate repository name * @param args * @param i argument index + * @param rootDirFile base repository directory */ private void validateRepName(String[] args, int i, File rootDirFile) { if (args.length < (i + 1)) { diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserAdmin.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserAdmin.java index 74dff0ec4e..a8c6fadc16 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserAdmin.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserAdmin.java @@ -25,6 +25,7 @@ import org.apache.logging.log4j.Logger; import ghidra.framework.store.local.LocalFileSystem; import ghidra.util.exception.DuplicateNameException; +import utilities.util.FileUtilities; /** * UserAdmin is an Application for generating administrative @@ -205,35 +206,17 @@ public class UserAdmin { log.info("Processing " + files.length + " queued commands"); for (File file : files) { - ArrayList cmdList = readCommands(file); - Iterator it = cmdList.iterator(); - while (it.hasNext()) { - processCommand(repositoryMgr, it.next()); + List cmdList = FileUtilities.getLines(file); + for (String cmdStr : cmdList) { + if (cmdStr.isBlank()) { + continue; + } + processCommand(repositoryMgr, cmdStr.trim()); } file.delete(); } } - /** - * Read all command strings contained within a file. - * @param cmdFile command file - * @return list of command strings - * @throws IOException - */ - private static ArrayList readCommands(File cmdFile) throws IOException { - ArrayList cmdList = new ArrayList<>(); - BufferedReader rdr = new BufferedReader(new FileReader(cmdFile)); - String cmd; - while ((cmd = rdr.readLine()) != null) { - if (cmd.length() == 0) { - continue; - } - cmdList.add(cmd.trim()); - } - rdr.close(); - return cmdList; - } - /** * Store a list of command strings to a new command file. * @param cmdList list of command strings diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserManager.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserManager.java index eb57ecbd25..15aaf9e0de 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserManager.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserManager.java @@ -17,6 +17,7 @@ package ghidra.server; import java.io.*; import java.util.*; +import java.util.regex.Pattern; import javax.security.auth.login.FailedLoginException; import javax.security.auth.x500.X500Principal; @@ -26,8 +27,7 @@ import org.apache.logging.log4j.Logger; import ghidra.framework.remote.User; import ghidra.framework.store.local.LocalFileSystem; -import ghidra.util.HashUtilities; -import ghidra.util.NumericUtilities; +import ghidra.util.*; import ghidra.util.exception.DuplicateNameException; /** @@ -183,7 +183,7 @@ public class UserManager { if (x500User != null) { dnLookupMap.put(x500User, entry); } - putUserList(); + writeUserList(); } /** @@ -260,7 +260,7 @@ public class UserManager { if (x500User != null) { dnLookupMap.put(x500User, entry); } - putUserList(); + writeUserList(); return true; } return false; @@ -338,7 +338,7 @@ public class UserManager { if (entry.x500User != null) { dnLookupMap.put(entry.x500User, entry); } - putUserList(); + writeUserList(); return true; } return false; @@ -426,7 +426,7 @@ public class UserManager { if (oldEntry.x500User != null) { dnLookupMap.remove(oldEntry.x500User); } - putUserList(); + writeUserList(); } } @@ -457,7 +457,7 @@ public class UserManager { } userListUpdateInProgress = true; try { - getUserList(); + readUserListIfNeeded(); clearExpiredPasswords(); if (processCmds) { UserAdmin.processCommands(repositoryMgr); @@ -488,21 +488,22 @@ public class UserManager { } } if (dataChanged) { - putUserList(); + writeUserList(); } } /** - * Read user data from file. + * Read user data from file if the timestamp on the file has changed. + * * @throws IOException */ - private void getUserList() throws IOException { + private void readUserListIfNeeded() throws IOException { long lastMod = userFile.lastModified(); if (lastUserListChange == lastMod) { if (lastMod == 0) { // Create empty file if it does not yet exist - putUserList(); + writeUserList(); } return; } @@ -546,58 +547,62 @@ public class UserManager { } } - private static void readUserList(File file, LinkedHashMap list, - HashMap x500LookupMap) throws IOException { + private static void readUserList(File file, Map usersIndexByName, + Map x500LookupMap) throws IOException { try (BufferedReader br = new BufferedReader(new FileReader(file))) { - String line = br.readLine(); - while (line != null) { - if (!line.startsWith("#")) { - try { - StringTokenizer st = new StringTokenizer(line, ":"); - UserEntry entry = new UserEntry(); - entry.username = st.nextToken(); + String line; + while ((line = br.readLine()) != null) { + if (line.startsWith("#")) { + continue; + } + try { + StringTokenizer st = new StringTokenizer(line, ":"); + UserEntry entry = new UserEntry(); + entry.username = st.nextToken(); + if (!isValidUserName(entry.username)) { + log.error("Invalid user name, skipping: " + entry.username); + continue; + } - // Password Hash + // Password Hash + if (st.hasMoreTokens()) { + entry.passwordHash = st.nextToken().toCharArray(); + + // Password Time if (st.hasMoreTokens()) { - entry.passwordHash = st.nextToken().toCharArray(); - - // Password Time - if (st.hasMoreTokens()) { - try { - String timeStr = st.nextToken(); - if ("*".equals(timeStr)) { - entry.passwordTime = NO_EXPIRATION; - } - else { - entry.passwordTime = NumericUtilities.parseHexLong(timeStr); - } + try { + String timeStr = st.nextToken(); + if ("*".equals(timeStr)) { + entry.passwordTime = NO_EXPIRATION; } - catch (NumberFormatException e) { - log.error("Invalid password time - forced expiration: " + - entry.username); - entry.passwordTime = 0; - } - - // Distinguished Name - if (st.hasMoreTokens()) { - String dn = st.nextToken(); - if (dn.length() > 0) { - entry.x500User = new X500Principal(dn); - } - + else { + entry.passwordTime = NumericUtilities.parseHexLong(timeStr); } } - } - list.put(entry.username, entry); - if (entry.x500User != null) { - x500LookupMap.put(entry.x500User, entry); + catch (NumberFormatException e) { + log.error( + "Invalid password time - forced expiration: " + entry.username); + entry.passwordTime = 0; + } + + // Distinguished Name + if (st.hasMoreTokens()) { + String dn = st.nextToken(); + if (dn.length() > 0) { + entry.x500User = new X500Principal(dn); + } + + } } } - catch (NoSuchElementException e) { - // skip entry + usersIndexByName.put(entry.username, entry); + if (entry.x500User != null) { + x500LookupMap.put(entry.x500User, entry); } } - line = br.readLine(); + catch (NoSuchElementException e) { + // skip entry + } } } } @@ -606,12 +611,9 @@ public class UserManager { * Write user data to file. * @throws IOException */ - private void putUserList() throws IOException { - BufferedWriter bw = new BufferedWriter(new FileWriter(userFile)); - try { - Iterator iter = userList.values().iterator(); - while (iter.hasNext()) { - UserEntry entry = iter.next(); + private void writeUserList() throws IOException { + try (BufferedWriter bw = new BufferedWriter(new FileWriter(userFile))) { + for (UserEntry entry : userList.values()) { bw.write(entry.username); bw.write(":"); if (entry.passwordHash != null) { @@ -634,13 +636,6 @@ public class UserManager { bw.newLine(); } } - finally { - try { - bw.close(); - } - catch (IOException e) { - } - } lastUserListChange = userFile.lastModified(); } @@ -743,4 +738,20 @@ public class UserManager { } } + /* + * Regex: matches if the entire string is alpha, digit, ".", "-", "_", fwd or back slash. + */ + private static final Pattern VALID_USERNAME_REGEX = Pattern.compile("[a-zA-Z0-9.-_/\\\\]+"); + + /** + * Ensures a name only contains valid characters and meets length limitations. + * + * @param s name string + * @return boolean true if valid name, false if not valid + */ + public static boolean isValidUserName(String s) { + return VALID_USERNAME_REGEX.matcher(s).matches() && + s.length() <= NamingUtilities.MAX_NAME_LENGTH; + } + } diff --git a/Ghidra/Framework/Utility/src/main/java/utilities/util/FileUtilities.java b/Ghidra/Framework/Utility/src/main/java/utilities/util/FileUtilities.java index 9d7a04ca6d..9651c11619 100644 --- a/Ghidra/Framework/Utility/src/main/java/utilities/util/FileUtilities.java +++ b/Ghidra/Framework/Utility/src/main/java/utilities/util/FileUtilities.java @@ -785,13 +785,13 @@ public final class FileUtilities { * @throws IOException if there are any issues reading the file */ public static List getLines(BufferedReader in) throws IOException { - List fileLines = new ArrayList<>(); try { + List fileLines = new ArrayList<>(); String line; while ((line = in.readLine()) != null) { fileLines.add(line); } - in.close(); + return fileLines; } finally { try { @@ -801,7 +801,6 @@ public final class FileUtilities { // don't care; we tried } } - return fileLines; } /**