From ef6993c636799a274e08a0fc2fec3e68ad9d6967 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Zubcevic?= Date: Sun, 5 Jul 2020 13:13:34 +0200 Subject: [PATCH] improving lesson due to issue #845 --- .../CatchAllConverter.java | 22 ------ .../ContactConverter.java | 52 ------------- .../VulnerableComponentsLesson.java | 58 +++------------ .../resources/i18n/WebGoatLabels.properties | 4 +- .../en/VulnerableComponents_content5a.adoc | 7 +- .../VulnerableComponentsLessonTest.java | 74 ++++++++++++------- 6 files changed, 64 insertions(+), 153 deletions(-) delete mode 100644 webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/CatchAllConverter.java delete mode 100644 webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/ContactConverter.java diff --git a/webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/CatchAllConverter.java b/webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/CatchAllConverter.java deleted file mode 100644 index 575f4d715..000000000 --- a/webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/CatchAllConverter.java +++ /dev/null @@ -1,22 +0,0 @@ -package org.owasp.webgoat.vulnerable_components; - -import com.thoughtworks.xstream.converters.Converter; -import com.thoughtworks.xstream.converters.MarshallingContext; -import com.thoughtworks.xstream.converters.UnmarshallingContext; -import com.thoughtworks.xstream.io.HierarchicalStreamReader; -import com.thoughtworks.xstream.io.HierarchicalStreamWriter; - -public class CatchAllConverter implements Converter { - - public boolean canConvert(Class clazz) { - return true; - } - - public void marshal(Object value, HierarchicalStreamWriter writer, MarshallingContext context) { - } - - public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) { - return null; - } - -} \ No newline at end of file diff --git a/webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/ContactConverter.java b/webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/ContactConverter.java deleted file mode 100644 index 200ce6f13..000000000 --- a/webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/ContactConverter.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * This file is part of WebGoat, an Open Web Application Security Project utility. For details, please see http://www.owasp.org/ - * - * Copyright (c) 2002 - 2019 Bruce Mayhew - * - * This program is free software; you can redistribute it and/or modify it under the terms of the - * GNU General Public License as published by the Free Software Foundation; either version 2 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without - * even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License along with this program; if - * not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA - * 02111-1307, USA. - * - * Getting Source ============== - * - * Source for this application is maintained at https://github.com/WebGoat/WebGoat, a repository for free software projects. - */ - -package org.owasp.webgoat.vulnerable_components; - -import com.thoughtworks.xstream.converters.Converter; -import com.thoughtworks.xstream.converters.MarshallingContext; -import com.thoughtworks.xstream.converters.UnmarshallingContext; -import com.thoughtworks.xstream.io.HierarchicalStreamReader; -import com.thoughtworks.xstream.io.HierarchicalStreamWriter; - -public class ContactConverter implements Converter { - - public boolean canConvert(Class clazz) { - return clazz.equals(Contact.class); - } - - public void marshal(Object value, HierarchicalStreamWriter writer, MarshallingContext context) { - Contact contact = (Contact) value; - writer.startNode("firstName"); - writer.setValue(contact.getFirstName()); - writer.endNode(); - } - - public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) { - Contact contact = new ContactImpl(); - reader.moveDown(); - contact.setFirstName(reader.getValue()); - reader.moveUp(); - return contact; - } - -} diff --git a/webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/VulnerableComponentsLesson.java b/webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/VulnerableComponentsLesson.java index b881aa4ab..7bba5afbf 100644 --- a/webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/VulnerableComponentsLesson.java +++ b/webgoat-lessons/vulnerable-components/src/main/java/org/owasp/webgoat/vulnerable_components/VulnerableComponentsLesson.java @@ -22,73 +22,35 @@ package org.owasp.webgoat.vulnerable_components; -import com.thoughtworks.xstream.XStream; -import com.thoughtworks.xstream.io.xml.DomDriver; - import org.apache.commons.lang3.StringUtils; import org.owasp.webgoat.assignments.AssignmentEndpoint; import org.owasp.webgoat.assignments.AssignmentHints; import org.owasp.webgoat.assignments.AttackResult; -import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.RestController; + +import com.thoughtworks.xstream.XStream; @RestController @AssignmentHints({"vulnerable.hint"}) public class VulnerableComponentsLesson extends AssignmentEndpoint { - - - /* - * - - - - - calc.exe - - - start - - - - -org.owasp.webgoat.vulnerable_components.Contact - - - - calc.exe - - - start - - - */ @PostMapping("/VulnerableComponents/attack1") public @ResponseBody AttackResult completed(@RequestParam String payload) { - XStream xstream = new XStream(/*new DomDriver()*/); + XStream xstream = new XStream(); xstream.setClassLoader(Contact.class.getClassLoader()); - - //xstream.processAnnotations(Contact.class); xstream.alias("contact", ContactImpl.class); - //xstream.aliasField("id", Contact.class, "id"); xstream.ignoreUnknownElements(); - //xstream.registerConverter(new ContactConverter()); - //xstream.registerConverter(new CatchAllConverter(), XStream.PRIORITY_VERY_LOW); - Contact contact = null; try { - - if (!StringUtils.isEmpty(payload)) { - //payload = payload.replace("contact ", " ", ">").replace(" <", "<"); } - System.out.println(payload); - contact = (Contact) xstream.fromXML(payload); - - } catch (Exception ex) { return failed(this).feedback("vulnerable-components.close").output(ex.getMessage()).build(); } @@ -96,10 +58,12 @@ public class VulnerableComponentsLesson extends AssignmentEndpoint { try { if (null!=contact) { contact.getFirstName();//trigger the example like https://x-stream.github.io/CVE-2013-7285.html + } + if (!(contact instanceof ContactImpl)) { + return success(this).feedback("vulnerable-components.success").build(); } } catch (Exception e) { - e.printStackTrace(); - return success(this).feedback("vulnerable-components.success").build(); + return success(this).feedback("vulnerable-components.success").output(e.getMessage()).build(); } return failed(this).feedback("vulnerable-components.fromXML").feedbackArgs(contact).build(); } diff --git a/webgoat-lessons/vulnerable-components/src/main/resources/i18n/WebGoatLabels.properties b/webgoat-lessons/vulnerable-components/src/main/resources/i18n/WebGoatLabels.properties index 07db2fc11..87c13198c 100644 --- a/webgoat-lessons/vulnerable-components/src/main/resources/i18n/WebGoatLabels.properties +++ b/webgoat-lessons/vulnerable-components/src/main/resources/i18n/WebGoatLabels.properties @@ -1,7 +1,7 @@ vulnerable-components.title=Vulnerable Components EnterYourName=Enter your Name Go!=Go! -vulnerable.hint=Here is some explanation of someone trying the exercise in an earlier version: https://www.youtube.com/watch?v=iWcRR2WcBFU -vulnerable-components.close=Trying to deserialize null object. +vulnerable.hint=Did you search for CVE-2013-728 and read https://x-stream.github.io/CVE-2013-7285.html +vulnerable-components.close=The payload send could not be deserialized to a Contact class. Please try again. vulnerable-components.success=You successfully tried to exploit the CVE-2013-7285 vulnerability vulnerable-components.fromXML=You created contact {0}. This means you did not exploit the remote code execution. \ No newline at end of file diff --git a/webgoat-lessons/vulnerable-components/src/main/resources/lessonPlans/en/VulnerableComponents_content5a.adoc b/webgoat-lessons/vulnerable-components/src/main/resources/lessonPlans/en/VulnerableComponents_content5a.adoc index e669e9825..ef89632e1 100644 --- a/webgoat-lessons/vulnerable-components/src/main/resources/lessonPlans/en/VulnerableComponents_content5a.adoc +++ b/webgoat-lessons/vulnerable-components/src/main/resources/lessonPlans/en/VulnerableComponents_content5a.adoc @@ -1,6 +1,6 @@ == Exploiting http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2013-7285[CVE-2013-7285] (XStream) -WebGoat Sends an XML document to add contacts to a contacts database. +WebGoat uses an XML document to add contacts to a contacts database. [source,xml] ---- @@ -11,5 +11,6 @@ WebGoat Sends an XML document to add contacts to a contacts database. ---- -For this example, we will let you enter the xml directly versus intercepting the request and modifying the data. You provide the XML representation of a contact and WebGoat will convert it a Contact object using `XStream.fromXML(xml)`. -So find information about the CVE vulnerability and sends some payload that triggers the vulnerability. +The java interface that you need for the exercise is: org.owasp.webgoat.vulnerable_components.Contact. +Start by sending the above contact to see what the normal response would be and then read the CVE vulnerability documentation (search the Internet) and try to trigger the vulnerability. +For this example, we will let you enter the XML directly versus intercepting the request and modifying the data. You provide the XML representation of a contact and WebGoat will convert it a Contact object using `XStream.fromXML(xml)`. diff --git a/webgoat-lessons/vulnerable-components/src/test/java/org/owasp/webgoat/vulnerable_components/VulnerableComponentsLessonTest.java b/webgoat-lessons/vulnerable-components/src/test/java/org/owasp/webgoat/vulnerable_components/VulnerableComponentsLessonTest.java index a56dc709e..8ae6d0736 100644 --- a/webgoat-lessons/vulnerable-components/src/test/java/org/owasp/webgoat/vulnerable_components/VulnerableComponentsLessonTest.java +++ b/webgoat-lessons/vulnerable-components/src/test/java/org/owasp/webgoat/vulnerable_components/VulnerableComponentsLessonTest.java @@ -22,37 +22,57 @@ package org.owasp.webgoat.vulnerable_components; -import org.junit.Before; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; -import org.owasp.webgoat.assignments.AssignmentEndpointTest; -import org.springframework.test.web.servlet.MockMvc; -import static org.mockito.Mockito.when; -import static org.springframework.test.web.servlet.setup.MockMvcBuilders.standaloneSetup; +import com.thoughtworks.xstream.XStream; +import com.thoughtworks.xstream.io.StreamException; -/** - * @author nbaars - * @date 2/7/17 - */ -@RunWith(MockitoJUnitRunner.class) -public class VulnerableComponentsLessonTest extends AssignmentEndpointTest { - - private MockMvc mockMvc; - - @Before - public void setup() { - VulnerableComponentsLesson vulnerableComponentsLesson = new VulnerableComponentsLesson(); - init(vulnerableComponentsLesson); - this.mockMvc = standaloneSetup(vulnerableComponentsLesson).build(); - } +public class VulnerableComponentsLessonTest { + String strangeContact = "\n" + + "org.owasp.webgoat.vulnerable_components.Contact\n" + + " \n" + + " \n" + + " \n" + + " calc.exe\n" + + " \n" + + " \n" + + " start\n" + + " \n" + + ""; + String contact = "\n"+ + ""; + @Test - public void success() throws Exception { -// mockMvc.perform(MockMvcRequestBuilders.post("/VulnerableComponents/attack1").content("Test")) -// .andExpect(status().isOk()) -// .andExpect(jsonPath("$.feedback", CoreMatchers.is(messages.getMessage("http-proxies.intercept.success")))) -// .andExpect(jsonPath("$.lessonCompleted", CoreMatchers.is(true))); + public void testTransformation() throws Exception { + XStream xstream = new XStream(); + xstream.setClassLoader(Contact.class.getClassLoader()); + xstream.alias("contact", ContactImpl.class); + xstream.ignoreUnknownElements(); + assertNotNull(xstream.fromXML(contact)); + } + + @Test + public void testIllegalTransformation() throws Exception { + XStream xstream = new XStream(); + xstream.setClassLoader(Contact.class.getClassLoader()); + xstream.alias("contact", ContactImpl.class); + xstream.ignoreUnknownElements(); + Exception e = assertThrows(RuntimeException.class, ()->((Contact)xstream.fromXML(strangeContact)).getFirstName()); + assertTrue(e.getCause().getMessage().contains("calc.exe")); + } + + @Test + public void testIllegalPayload() throws Exception { + XStream xstream = new XStream(); + xstream.setClassLoader(Contact.class.getClassLoader()); + xstream.alias("contact", ContactImpl.class); + xstream.ignoreUnknownElements(); + Exception e = assertThrows(StreamException.class, ()->((Contact)xstream.fromXML("bullssjfs")).getFirstName()); + assertTrue(e.getCause().getMessage().contains("START_DOCUMENT")); } } \ No newline at end of file