From d20c88168d0c0f203811fcfe0d0f847cef1bc8f1 Mon Sep 17 00:00:00 2001 From: k-tamura Date: Sat, 27 May 2017 14:58:02 +0900 Subject: [PATCH] Code refactoring --- .../org/t246osslab/easybuggy/core/dao/DBClient.java | 3 ++- .../t246osslab/easybuggy/core/utils/EmailUtils.java | 7 ++++--- .../performance/SlowRegularExpressionServlet.java | 3 ++- .../easybuggy/troubles/DBConnectionLeakServlet.java | 5 +++-- .../easybuggy/vulnerabilities/CSRFServlet.java | 5 +++-- .../vulnerabilities/ClickJackingServlet.java | 3 ++- .../vulnerabilities/CodeInjectionServlet.java | 3 ++- .../vulnerabilities/MailHeaderInjectionServlet.java | 3 ++- .../vulnerabilities/NullByteInjectionServlet.java | 3 ++- .../OGNLExpressionInjectionServlet.java | 11 ++++++----- .../vulnerabilities/SQLInjectionServlet.java | 3 ++- .../UnrestrictedExtensionUploadServlet.java | 7 ++++--- .../UnrestrictedSizeUploadServlet.java | 13 +++++++------ .../easybuggy/vulnerabilities/XEEandXXEServlet.java | 4 ++-- .../easybuggy/vulnerabilities/XSSServlet.java | 2 +- 15 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/t246osslab/easybuggy/core/dao/DBClient.java b/src/main/java/org/t246osslab/easybuggy/core/dao/DBClient.java index 6c2e09d9..85918b1f 100644 --- a/src/main/java/org/t246osslab/easybuggy/core/dao/DBClient.java +++ b/src/main/java/org/t246osslab/easybuggy/core/dao/DBClient.java @@ -6,6 +6,7 @@ import java.sql.Statement; import org.apache.commons.lang.RandomStringUtils; +import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.t246osslab.easybuggy.core.utils.ApplicationUtils; @@ -49,7 +50,7 @@ private DBClient() { public static Connection getConnection() throws SQLException { final String dbDriver = ApplicationUtils.getDatabaseDriver(); final String dbUrl = ApplicationUtils.getDatabaseURL(); - if (dbDriver != null && !"".equals(dbDriver)) { + if (!StringUtils.isBlank(dbDriver)) { try { Class.forName(dbDriver); } catch (ClassNotFoundException e) { diff --git a/src/main/java/org/t246osslab/easybuggy/core/utils/EmailUtils.java b/src/main/java/org/t246osslab/easybuggy/core/utils/EmailUtils.java index 6094b65f..3b06017e 100644 --- a/src/main/java/org/t246osslab/easybuggy/core/utils/EmailUtils.java +++ b/src/main/java/org/t246osslab/easybuggy/core/utils/EmailUtils.java @@ -19,6 +19,7 @@ import javax.mail.internet.MimeMessage; import javax.mail.internet.MimeMultipart; +import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -36,9 +37,9 @@ private EmailUtils() { } public static boolean isReadyToSendEmail() { - if (ApplicationUtils.getSmtpHost() == null || "".equals(ApplicationUtils.getSmtpHost()) - || ApplicationUtils.getSmtpPort() == null || "".equals(ApplicationUtils.getSmtpPort()) - || ApplicationUtils.getAdminAddress() == null || "".equals(ApplicationUtils.getAdminAddress())) { + if (StringUtils.isBlank(ApplicationUtils.getSmtpHost()) + || StringUtils.isBlank(ApplicationUtils.getSmtpPort()) + || StringUtils.isBlank(ApplicationUtils.getAdminAddress())) { return false; } return true; diff --git a/src/main/java/org/t246osslab/easybuggy/performance/SlowRegularExpressionServlet.java b/src/main/java/org/t246osslab/easybuggy/performance/SlowRegularExpressionServlet.java index e899437b..4134449c 100644 --- a/src/main/java/org/t246osslab/easybuggy/performance/SlowRegularExpressionServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/performance/SlowRegularExpressionServlet.java @@ -12,6 +12,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.t246osslab.easybuggy.core.utils.HTTPResponseCreator; @@ -42,7 +43,7 @@ protected void service(HttpServletRequest req, HttpServletResponse res) throws S bodyHtml.append(""); bodyHtml.append("

"); - if (word != null && !"".equals(word)) { + if (!StringUtils.isBlank(word)) { Date startDate = new Date(); log.info("Start Date: {}", startDate.toString()); Pattern compile = Pattern.compile("^([a-z0-9]+[-]{0,1}){1,100}$"); diff --git a/src/main/java/org/t246osslab/easybuggy/troubles/DBConnectionLeakServlet.java b/src/main/java/org/t246osslab/easybuggy/troubles/DBConnectionLeakServlet.java index 77624234..10392378 100644 --- a/src/main/java/org/t246osslab/easybuggy/troubles/DBConnectionLeakServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/troubles/DBConnectionLeakServlet.java @@ -12,6 +12,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.t246osslab.easybuggy.core.dao.DBClient; @@ -33,7 +34,7 @@ protected void service(HttpServletRequest req, HttpServletResponse res) throws S final String dbUrl = ApplicationUtils.getDatabaseURL(); final String dbDriver = ApplicationUtils.getDatabaseDriver(); - if (dbDriver != null && !"".equals(dbDriver)) { + if (!StringUtils.isBlank(dbDriver)) { try { Class.forName(dbDriver); } catch (Exception e) { @@ -41,7 +42,7 @@ protected void service(HttpServletRequest req, HttpServletResponse res) throws S } } bodyHtml.append(selectUsers(locale)); - if (dbUrl == null || "".equals(dbUrl) || dbUrl.startsWith("jdbc:derby:memory:")) { + if (StringUtils.isBlank(dbUrl) || dbUrl.startsWith("jdbc:derby:memory:")) { bodyHtml.append(MessageUtils.getInfoMsg("msg.note.not.use.ext.db", locale)); } else { bodyHtml.append(MessageUtils.getInfoMsg("msg.db.connection.leak.occur", locale)); diff --git a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/CSRFServlet.java b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/CSRFServlet.java index e20f5557..0249398a 100644 --- a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/CSRFServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/CSRFServlet.java @@ -10,6 +10,7 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +import org.apache.commons.lang.StringUtils; import org.apache.directory.shared.ldap.entry.ModificationOperation; import org.apache.directory.shared.ldap.entry.client.ClientModification; import org.apache.directory.shared.ldap.entry.client.DefaultClientAttribute; @@ -59,7 +60,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse res) throws Se } String userid = (String) session.getAttribute("userid"); String password = req.getParameter("password"); - if (userid != null && password != null && !"".equals(userid) && !"".equals(password) && password.length() >= 8) { + if (!StringUtils.isBlank(userid) && !StringUtils.isBlank(password) && password.length() >= 8) { try { DefaultClientAttribute entryAttribute = new DefaultClientAttribute("userPassword", ESAPI.encoder() .encodeForLDAP(password.trim())); @@ -87,7 +88,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse res) throws Se doGet(req, res); } } else { - if (password == null || "".equals(password) || password.length() < 8) { + if (StringUtils.isBlank(password) || password.length() < 8) { req.setAttribute("errorMessage", MessageUtils.getErrMsg("msg.passwd.is.too.short", locale)); } else { req.setAttribute("errorMessage", MessageUtils.getErrMsg("msg.unknown.exception.occur", diff --git a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/ClickJackingServlet.java b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/ClickJackingServlet.java index 46af52ea..f41f41e8 100644 --- a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/ClickJackingServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/ClickJackingServlet.java @@ -12,6 +12,7 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +import org.apache.commons.lang.StringUtils; import org.apache.directory.shared.ldap.entry.ModificationOperation; import org.apache.directory.shared.ldap.entry.client.ClientModification; import org.apache.directory.shared.ldap.entry.client.DefaultClientAttribute; @@ -61,7 +62,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse res) throws Se } String userid = (String) session.getAttribute("userid"); String mail = req.getParameter("mail"); - if (mail != null && !"".equals(mail) && isValidEmailAddress(mail)) { + if (!StringUtils.isBlank(mail) && isValidEmailAddress(mail)) { try { DefaultClientAttribute entryAttribute = new DefaultClientAttribute("mail", ESAPI.encoder() .encodeForLDAP(mail.trim())); diff --git a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/CodeInjectionServlet.java b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/CodeInjectionServlet.java index fc53085f..947a482c 100644 --- a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/CodeInjectionServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/CodeInjectionServlet.java @@ -12,6 +12,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang.StringUtils; import org.owasp.esapi.ESAPI; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,7 +46,7 @@ protected void service(HttpServletRequest req, HttpServletResponse res) throws S bodyHtml.append(""); bodyHtml.append("

"); - if (jsonString != null && !"".equals(jsonString)) { + if (!StringUtils.isBlank(jsonString)) { jsonString = jsonString.replaceAll(" ", ""); jsonString = jsonString.replaceAll("\r\n", ""); jsonString = jsonString.replaceAll("\n", ""); diff --git a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/MailHeaderInjectionServlet.java b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/MailHeaderInjectionServlet.java index 18a053de..7fbda763 100644 --- a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/MailHeaderInjectionServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/MailHeaderInjectionServlet.java @@ -17,6 +17,7 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.Part; +import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.t246osslab.easybuggy.core.utils.EmailUtils; @@ -98,7 +99,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse res) throws Se String mail = req.getParameter("mail"); String subject = req.getParameter("subject"); String content = req.getParameter("content"); - if (subject == null || "".equals(subject.trim()) || content == null || "".equals(content.trim())) { + if (StringUtils.isBlank(subject) || StringUtils.isBlank(content)) { resultMessage = MessageUtils.getMsg("msg.mail.is.empty", locale); doGet(req, res); return; diff --git a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/NullByteInjectionServlet.java b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/NullByteInjectionServlet.java index f60aba51..03224b75 100644 --- a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/NullByteInjectionServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/NullByteInjectionServlet.java @@ -14,6 +14,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.t246osslab.easybuggy.core.utils.Closer; @@ -37,7 +38,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse res) throws Ser bodyHtml.append("

" + MessageUtils.getInfoMsg("msg.note.null.byte.injection", locale) + "

"); try { String fileName = req.getParameter("fileName"); - if (fileName == null || "".equals(fileName)) { + if (StringUtils.isBlank(fileName)) { HTTPResponseCreator.createSimpleResponse(req, res, MessageUtils.getMsg("title.guide.download", locale), bodyHtml.toString()); return; diff --git a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/OGNLExpressionInjectionServlet.java b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/OGNLExpressionInjectionServlet.java index 3e3d3c3f..02d16c91 100644 --- a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/OGNLExpressionInjectionServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/OGNLExpressionInjectionServlet.java @@ -9,10 +9,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import ognl.Ognl; -import ognl.OgnlContext; -import ognl.OgnlException; - +import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.math.NumberUtils; import org.owasp.esapi.ESAPI; import org.slf4j.Logger; @@ -20,6 +17,10 @@ import org.t246osslab.easybuggy.core.utils.HTTPResponseCreator; import org.t246osslab.easybuggy.core.utils.MessageUtils; +import ognl.Ognl; +import ognl.OgnlContext; +import ognl.OgnlException; + @SuppressWarnings("serial") @WebServlet(urlPatterns = { "/ognleijc" }) public class OGNLExpressionInjectionServlet extends HttpServlet { @@ -36,7 +37,7 @@ protected void service(HttpServletRequest req, HttpServletResponse res) throws S boolean isValid = true; OgnlContext ctx = new OgnlContext(); String expression = req.getParameter("expression"); - if (expression == null || "".equals(expression)) { + if (StringUtils.isBlank(expression)) { isValid = false; } else { try { diff --git a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/SQLInjectionServlet.java b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/SQLInjectionServlet.java index 620599aa..31f9446b 100644 --- a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/SQLInjectionServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/SQLInjectionServlet.java @@ -12,6 +12,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.t246osslab.easybuggy.core.dao.DBClient; @@ -45,7 +46,7 @@ protected void service(HttpServletRequest req, HttpServletResponse res) throws S bodyHtml.append(""); bodyHtml.append("

"); - if (name != null && password != null && !"".equals(name) && !"".equals(password) && password.length() >= 8) { + if (!StringUtils.isBlank(name) && !StringUtils.isBlank(password) && password.length() >= 8) { bodyHtml.append(selectUsers(name, password, req)); } else { bodyHtml.append(MessageUtils.getMsg("msg.warn.enter.name.and.passwd", locale)); diff --git a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/UnrestrictedExtensionUploadServlet.java b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/UnrestrictedExtensionUploadServlet.java index d274db54..a3f439d7 100644 --- a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/UnrestrictedExtensionUploadServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/UnrestrictedExtensionUploadServlet.java @@ -18,6 +18,7 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.Part; +import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.t246osslab.easybuggy.core.utils.Closer; @@ -84,7 +85,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse res) throws Se } try { String fileName = getFileName(filePart); - if (fileName == null || "".equals(fileName)) { + if (StringUtils.isBlank(fileName)) { doGet(req, res); return; } @@ -118,14 +119,14 @@ protected void doPost(HttpServletRequest req, HttpServletResponse res) throws Se StringBuilder bodyHtml = new StringBuilder(); if (isConverted) { bodyHtml.append(MessageUtils.getMsg("msg.convert.grayscale.complete", locale)); + bodyHtml.append("

"); } else { - bodyHtml.append(MessageUtils.getMsg("msg.convert.grayscale.fail", locale)); + bodyHtml.append(MessageUtils.getErrMsg("msg.convert.grayscale.fail", locale)); } if (isConverted) { bodyHtml.append("

"); bodyHtml.append(""); } - bodyHtml.append("

"); bodyHtml.append(""); HTTPResponseCreator.createSimpleResponse(req, res, MessageUtils.getMsg("title.unrestricted.extension.upload", locale), diff --git a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/UnrestrictedSizeUploadServlet.java b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/UnrestrictedSizeUploadServlet.java index f3df1077..6dfdb558 100644 --- a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/UnrestrictedSizeUploadServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/UnrestrictedSizeUploadServlet.java @@ -19,6 +19,7 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.Part; +import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.t246osslab.easybuggy.core.utils.Closer; @@ -77,7 +78,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse res) throws Se final Part filePart = req.getPart("file"); try { String fileName = getFileName(filePart); - if (fileName == null || "".equals(fileName)) { + if (StringUtils.isBlank(fileName)) { doGet(req, res); return; } else if (!isImageFile(fileName)) { @@ -102,8 +103,8 @@ protected void doPost(HttpServletRequest req, HttpServletResponse res) throws Se try { // Reverse the color of the upload image - if(!isConverted){ - revereColor(new File(savePath + File.separator + fileName).getAbsolutePath()); + if (!isConverted) { + reverseColor(new File(savePath + File.separator + fileName).getAbsolutePath()); isConverted = true; } } catch (Exception e) { @@ -114,14 +115,14 @@ protected void doPost(HttpServletRequest req, HttpServletResponse res) throws Se StringBuilder bodyHtml = new StringBuilder(); if (isConverted) { bodyHtml.append(MessageUtils.getMsg("msg.reverse.color.complete", locale)); + bodyHtml.append("

"); } else { - bodyHtml.append(MessageUtils.getMsg("msg.reverse.color.fail", locale)); + bodyHtml.append(MessageUtils.getErrMsg("msg.reverse.color.fail", locale)); } if (isConverted) { bodyHtml.append("

"); bodyHtml.append(""); } - bodyHtml.append("

"); bodyHtml.append(""); HTTPResponseCreator.createSimpleResponse(req, res, MessageUtils.getMsg("title.unrestricted.size.upload", locale), @@ -151,7 +152,7 @@ private String getFileName(final Part part) { } // Reverse the color of the image file - private void revereColor(String fileName) throws IOException { + private void reverseColor(String fileName) throws IOException { BufferedImage image = ImageIO.read(new File(fileName)); WritableRaster raster = image.getRaster(); int[] pixelBuffer = new int[raster.getNumDataElements()]; diff --git a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/XEEandXXEServlet.java b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/XEEandXXEServlet.java index f62c38c8..2e418a41 100644 --- a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/XEEandXXEServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/XEEandXXEServlet.java @@ -25,6 +25,7 @@ import javax.xml.parsers.SAXParserFactory; import org.apache.commons.lang.RandomStringUtils; +import org.apache.commons.lang.StringUtils; import org.owasp.esapi.ESAPI; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -161,7 +162,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse res) throws Se } try { String fileName = getFileName(filePart); - if (fileName == null || "".equals(fileName)) { + if (StringUtils.isBlank(fileName)) { doGet(req, res); return; } else if (!fileName.endsWith(".xml")) { @@ -236,7 +237,6 @@ protected void doPost(HttpServletRequest req, HttpServletResponse res) throws Se HTTPResponseCreator.createSimpleResponse(req, res, MessageUtils.getMsg("title.xxe", locale), bodyHtml.toString()); } - } catch (Exception e) { log.error("Exception occurs: ", e); } finally { diff --git a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/XSSServlet.java b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/XSSServlet.java index 1e2b2694..f711cda6 100644 --- a/src/main/java/org/t246osslab/easybuggy/vulnerabilities/XSSServlet.java +++ b/src/main/java/org/t246osslab/easybuggy/vulnerabilities/XSSServlet.java @@ -38,7 +38,7 @@ protected void service(HttpServletRequest req, HttpServletResponse res) throws S bodyHtml.append(""); bodyHtml.append("

"); - if (string != null && !"".equals(string)) { + if (!StringUtils.isBlank(string)) { // Reverse the given string String reversedName = StringUtils.reverse(string); bodyHtml.append(MessageUtils.getMsg("label.reversed.string", locale) + " : "