From 3c275a584238d4b43878d9546fdfa6282e506a8e Mon Sep 17 00:00:00 2001 From: gracekarina Date: Wed, 26 Dec 2018 14:59:27 -0500 Subject: [PATCH 1/4] test and solution to sercurity issue --- .../io/swagger/parser/Swagger20Parser.java | 7 +- .../io/swagger/parser/SwaggerParserTest.java | 7 + .../src/test/resources/malicious.yaml | 603 ++++++++++++++++++ 3 files changed, 614 insertions(+), 3 deletions(-) create mode 100644 modules/swagger-parser/src/test/resources/malicious.yaml diff --git a/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java b/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java index b5b845565d..cccd0d2284 100644 --- a/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java +++ b/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java @@ -113,14 +113,15 @@ public Swagger read(String location, List auths) throws IOEx private Swagger convertToSwagger(String data) throws IOException { if (data != null) { - JsonNode rootNode; + JsonNode rootNode = null; if (data.trim().startsWith("{")) { ObjectMapper mapper = Json.mapper(); rootNode = mapper.readTree(data); - } else { - rootNode = DeserializationUtils.readYamlTree(data); + } else if (data.trim().startsWith("swagger") ||data.trim().startsWith("---")|| data.trim().startsWith("#")) { + rootNode = DeserializationUtils.readYamlTree(data); } + if (System.getProperty("debugParser") != null) { LOGGER.info("\n\nSwagger Tree: \n" + ReflectionToStringBuilder.toString(rootNode, ToStringStyle.MULTI_LINE_STYLE) + "\n\n"); diff --git a/modules/swagger-parser/src/test/java/io/swagger/parser/SwaggerParserTest.java b/modules/swagger-parser/src/test/java/io/swagger/parser/SwaggerParserTest.java index 83f54e0542..a91368eefc 100644 --- a/modules/swagger-parser/src/test/java/io/swagger/parser/SwaggerParserTest.java +++ b/modules/swagger-parser/src/test/java/io/swagger/parser/SwaggerParserTest.java @@ -57,6 +57,13 @@ public class SwaggerParserTest { + @Test + public void testIssueSecurity() { + Swagger swagger = new SwaggerParser().read("malicious.yaml"); + assertNull(swagger); + } + + @Test public void testIssueRelativeRefs2(){ String location = "exampleSpecs/specs/my-domain/test-api/v1/test-api-swagger_v1.json"; diff --git a/modules/swagger-parser/src/test/resources/malicious.yaml b/modules/swagger-parser/src/test/resources/malicious.yaml new file mode 100644 index 0000000000..8659fd378f --- /dev/null +++ b/modules/swagger-parser/src/test/resources/malicious.yaml @@ -0,0 +1,603 @@ +&id001 +*id001: poop +r: &id200 + a: + a: &id198 + a: &id196 + a: &id194 + a: &id192 + a: &id190 + a: &id188 + a: &id186 + a: &id184 + a: &id182 + a: &id180 + a: &id178 + a: &id176 + a: &id174 + a: &id172 + a: &id170 + a: &id168 + a: &id166 + a: &id164 + a: &id162 + a: &id160 + a: &id158 + a: &id156 + a: &id154 + a: &id152 + a: &id150 + a: &id148 + a: &id146 + a: &id144 + a: &id142 + a: &id140 + a: &id138 + a: &id136 + a: &id134 + a: &id132 + a: &id130 + a: &id128 + a: &id126 + a: &id124 + a: &id122 + a: &id120 + a: &id118 + a: &id116 + a: &id114 + a: &id112 + a: &id110 + a: &id108 + a: &id106 + a: &id104 + a: &id102 + a: &id100 + a: &id098 + a: &id096 + a: &id094 + a: &id092 + a: &id090 + a: &id088 + a: &id086 + a: &id084 + a: &id082 + a: &id080 + a: &id078 + a: &id076 + a: &id074 + a: &id072 + a: &id070 + a: &id068 + a: &id066 + a: &id064 + a: &id062 + a: &id060 + a: &id058 + a: &id056 + a: &id054 + a: &id052 + a: &id050 + a: &id048 + a: &id046 + a: &id044 + a: &id042 + a: &id040 + a: &id038 + a: &id036 + a: &id034 + a: &id032 + a: &id030 + a: &id028 + a: &id026 + a: &id024 + a: &id022 + a: &id020 + a: &id018 + a: &id016 + a: &id014 + a: &id012 + a: &id010 + a: &id008 + a: &id006 + a: &id004 + a: &id002 { + foo: '1'} + b: &id003 { + bar: '2'} + foo: '1' + b: &id005 + a: *id002 + bar: '2' + b: *id003 + foo: '1' + b: &id007 + a: *id004 + bar: '2' + b: *id005 + foo: '1' + b: &id009 + a: *id006 + bar: '2' + b: *id007 + foo: '1' + b: &id011 + a: *id008 + bar: '2' + b: *id009 + foo: '1' + b: &id013 + a: *id010 + bar: '2' + b: *id011 + foo: '1' + b: &id015 + a: *id012 + bar: '2' + b: *id013 + foo: '1' + b: &id017 + a: *id014 + bar: '2' + b: *id015 + foo: '1' + b: &id019 + a: *id016 + bar: '2' + b: *id017 + foo: '1' + b: &id021 + a: *id018 + bar: '2' + b: *id019 + foo: '1' + b: &id023 + a: *id020 + bar: '2' + b: *id021 + foo: '1' + b: &id025 + a: *id022 + bar: '2' + b: *id023 + foo: '1' + b: &id027 + a: *id024 + bar: '2' + b: *id025 + foo: '1' + b: &id029 + a: *id026 + bar: '2' + b: *id027 + foo: '1' + b: &id031 + a: *id028 + bar: '2' + b: *id029 + foo: '1' + b: &id033 + a: *id030 + bar: '2' + b: *id031 + foo: '1' + b: &id035 + a: *id032 + bar: '2' + b: *id033 + foo: '1' + b: &id037 + a: *id034 + bar: '2' + b: *id035 + foo: '1' + b: &id039 + a: *id036 + bar: '2' + b: *id037 + foo: '1' + b: &id041 + a: *id038 + bar: '2' + b: *id039 + foo: '1' + b: &id043 + a: *id040 + bar: '2' + b: *id041 + foo: '1' + b: &id045 + a: *id042 + bar: '2' + b: *id043 + foo: '1' + b: &id047 + a: *id044 + bar: '2' + b: *id045 + foo: '1' + b: &id049 + a: *id046 + bar: '2' + b: *id047 + foo: '1' + b: &id051 + a: *id048 + bar: '2' + b: *id049 + foo: '1' + b: &id053 + a: *id050 + bar: '2' + b: *id051 + foo: '1' + b: &id055 + a: *id052 + bar: '2' + b: *id053 + foo: '1' + b: &id057 + a: *id054 + bar: '2' + b: *id055 + foo: '1' + b: &id059 + a: *id056 + bar: '2' + b: *id057 + foo: '1' + b: &id061 + a: *id058 + bar: '2' + b: *id059 + foo: '1' + b: &id063 + a: *id060 + bar: '2' + b: *id061 + foo: '1' + b: &id065 + a: *id062 + bar: '2' + b: *id063 + foo: '1' + b: &id067 + a: *id064 + bar: '2' + b: *id065 + foo: '1' + b: &id069 + a: *id066 + bar: '2' + b: *id067 + foo: '1' + b: &id071 + a: *id068 + bar: '2' + b: *id069 + foo: '1' + b: &id073 + a: *id070 + bar: '2' + b: *id071 + foo: '1' + b: &id075 + a: *id072 + bar: '2' + b: *id073 + foo: '1' + b: &id077 + a: *id074 + bar: '2' + b: *id075 + foo: '1' + b: &id079 + a: *id076 + bar: '2' + b: *id077 + foo: '1' + b: &id081 + a: *id078 + bar: '2' + b: *id079 + foo: '1' + b: &id083 + a: *id080 + bar: '2' + b: *id081 + foo: '1' + b: &id085 + a: *id082 + bar: '2' + b: *id083 + foo: '1' + b: &id087 + a: *id084 + bar: '2' + b: *id085 + foo: '1' + b: &id089 + a: *id086 + bar: '2' + b: *id087 + foo: '1' + b: &id091 + a: *id088 + bar: '2' + b: *id089 + foo: '1' + b: &id093 + a: *id090 + bar: '2' + b: *id091 + foo: '1' + b: &id095 + a: *id092 + bar: '2' + b: *id093 + foo: '1' + b: &id097 + a: *id094 + bar: '2' + b: *id095 + foo: '1' + b: &id099 + a: *id096 + bar: '2' + b: *id097 + foo: '1' + b: &id101 + a: *id098 + bar: '2' + b: *id099 + foo: '1' + b: &id103 + a: *id100 + bar: '2' + b: *id101 + foo: '1' + b: &id105 + a: *id102 + bar: '2' + b: *id103 + foo: '1' + b: &id107 + a: *id104 + bar: '2' + b: *id105 + foo: '1' + b: &id109 + a: *id106 + bar: '2' + b: *id107 + foo: '1' + b: &id111 + a: *id108 + bar: '2' + b: *id109 + foo: '1' + b: &id113 + a: *id110 + bar: '2' + b: *id111 + foo: '1' + b: &id115 + a: *id112 + bar: '2' + b: *id113 + foo: '1' + b: &id117 + a: *id114 + bar: '2' + b: *id115 + foo: '1' + b: &id119 + a: *id116 + bar: '2' + b: *id117 + foo: '1' + b: &id121 + a: *id118 + bar: '2' + b: *id119 + foo: '1' + b: &id123 + a: *id120 + bar: '2' + b: *id121 + foo: '1' + b: &id125 + a: *id122 + bar: '2' + b: *id123 + foo: '1' + b: &id127 + a: *id124 + bar: '2' + b: *id125 + foo: '1' + b: &id129 + a: *id126 + bar: '2' + b: *id127 + foo: '1' + b: &id131 + a: *id128 + bar: '2' + b: *id129 + foo: '1' + b: &id133 + a: *id130 + bar: '2' + b: *id131 + foo: '1' + b: &id135 + a: *id132 + bar: '2' + b: *id133 + foo: '1' + b: &id137 + a: *id134 + bar: '2' + b: *id135 + foo: '1' + b: &id139 + a: *id136 + bar: '2' + b: *id137 + foo: '1' + b: &id141 + a: *id138 + bar: '2' + b: *id139 + foo: '1' + b: &id143 + a: *id140 + bar: '2' + b: *id141 + foo: '1' + b: &id145 + a: *id142 + bar: '2' + b: *id143 + foo: '1' + b: &id147 + a: *id144 + bar: '2' + b: *id145 + foo: '1' + b: &id149 + a: *id146 + bar: '2' + b: *id147 + foo: '1' + b: &id151 + a: *id148 + bar: '2' + b: *id149 + foo: '1' + b: &id153 + a: *id150 + bar: '2' + b: *id151 + foo: '1' + b: &id155 + a: *id152 + bar: '2' + b: *id153 + foo: '1' + b: &id157 + a: *id154 + bar: '2' + b: *id155 + foo: '1' + b: &id159 + a: *id156 + bar: '2' + b: *id157 + foo: '1' + b: &id161 + a: *id158 + bar: '2' + b: *id159 + foo: '1' + b: &id163 + a: *id160 + bar: '2' + b: *id161 + foo: '1' + b: &id165 + a: *id162 + bar: '2' + b: *id163 + foo: '1' + b: &id167 + a: *id164 + bar: '2' + b: *id165 + foo: '1' + b: &id169 + a: *id166 + bar: '2' + b: *id167 + foo: '1' + b: &id171 + a: *id168 + bar: '2' + b: *id169 + foo: '1' + b: &id173 + a: *id170 + bar: '2' + b: *id171 + foo: '1' + b: &id175 + a: *id172 + bar: '2' + b: *id173 + foo: '1' + b: &id177 + a: *id174 + bar: '2' + b: *id175 + foo: '1' + b: &id179 + a: *id176 + bar: '2' + b: *id177 + foo: '1' + b: &id181 + a: *id178 + bar: '2' + b: *id179 + foo: '1' + b: &id183 + a: *id180 + bar: '2' + b: *id181 + foo: '1' + b: &id185 + a: *id182 + bar: '2' + b: *id183 + foo: '1' + b: &id187 + a: *id184 + bar: '2' + b: *id185 + foo: '1' + b: &id189 + a: *id186 + bar: '2' + b: *id187 + foo: '1' + b: &id191 + a: *id188 + bar: '2' + b: *id189 + foo: '1' + b: &id193 + a: *id190 + bar: '2' + b: *id191 + foo: '1' + b: &id195 + a: *id192 + bar: '2' + b: *id193 + foo: '1' + b: &id197 + a: *id194 + bar: '2' + b: *id195 + foo: '1' + b: &id199 + a: *id196 + bar: '2' + b: *id197 + foo: '1' + b: + a: *id198 + bar: '2' + b: *id199 +j: *id200 + From 8cdaca4ed96536f5a1afe3859dedbd0df9111fbd Mon Sep 17 00:00:00 2001 From: gracekarina Date: Wed, 26 Dec 2018 19:23:46 -0500 Subject: [PATCH 2/4] fixing issue in readWithinfo method --- .../main/java/io/swagger/parser/Swagger20Parser.java | 7 +++++-- .../io/swagger/parser/util/SwaggerDeserializer.java | 10 ++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java b/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java index cccd0d2284..69e177924f 100644 --- a/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java +++ b/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java @@ -56,14 +56,17 @@ public SwaggerDeserializationResult readWithInfo(String location, List operationIDs = new HashSet<>(); public SwaggerDeserializationResult deserialize(JsonNode rootNode) { + SwaggerDeserializationResult result = new SwaggerDeserializationResult(); ParseResult rootParse = new ParseResult(); - - Swagger swagger = parseRoot(rootNode, rootParse); - result.setSwagger(swagger); - result.setMessages(rootParse.getMessages()); + if(rootNode != null) { + Swagger swagger = parseRoot(rootNode, rootParse); + result.setSwagger(swagger); + result.setMessages(rootParse.getMessages()); + } return result; } From d7bc6bd352770d0ba871ac043a59c7e15f0849e8 Mon Sep 17 00:00:00 2001 From: gracekarina Date: Fri, 28 Dec 2018 19:27:31 -0500 Subject: [PATCH 3/4] changing validation to pattern --- .../main/java/io/swagger/parser/Swagger20Parser.java | 8 ++++---- .../io/swagger/parser/util/DeserializationUtils.java | 12 +++++++++++- .../io/swagger/parser/util/SwaggerDeserializer.java | 11 +++++------ 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java b/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java index 69e177924f..e079f14fbf 100644 --- a/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java +++ b/modules/swagger-parser/src/main/java/io/swagger/parser/Swagger20Parser.java @@ -56,11 +56,11 @@ public SwaggerDeserializationResult readWithInfo(String location, List auths) throws IOEx private Swagger convertToSwagger(String data) throws IOException { if (data != null) { - JsonNode rootNode = null; + JsonNode rootNode; if (data.trim().startsWith("{")) { ObjectMapper mapper = Json.mapper(); rootNode = mapper.readTree(data); - } else if (data.trim().startsWith("swagger") ||data.trim().startsWith("---")|| data.trim().startsWith("#")) { + } else { rootNode = DeserializationUtils.readYamlTree(data); } diff --git a/modules/swagger-parser/src/main/java/io/swagger/parser/util/DeserializationUtils.java b/modules/swagger-parser/src/main/java/io/swagger/parser/util/DeserializationUtils.java index dd665edb41..afcf0d0115 100644 --- a/modules/swagger-parser/src/main/java/io/swagger/parser/util/DeserializationUtils.java +++ b/modules/swagger-parser/src/main/java/io/swagger/parser/util/DeserializationUtils.java @@ -10,6 +10,8 @@ import org.yaml.snakeyaml.constructor.SafeConstructor; import java.io.IOException; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * Created by russellb337 on 7/14/15. @@ -62,12 +64,20 @@ private static boolean isJson(String contents) { } public static JsonNode readYamlTree(String contents) { + String pattern = "&"; + Pattern r = Pattern.compile(pattern); + Matcher m = r.matcher(contents); org.yaml.snakeyaml.Yaml yaml = new org.yaml.snakeyaml.Yaml(new SafeConstructor()); - return Json.mapper().convertValue(yaml.load(contents), JsonNode.class); + if (!m.lookingAt()) { + return Json.mapper().convertValue(yaml.load(contents), JsonNode.class); + + } + return null; } public static T readYamlValue(String contents, Class expectedType) { org.yaml.snakeyaml.Yaml yaml = new org.yaml.snakeyaml.Yaml(new SafeConstructor()); return Json.mapper().convertValue(yaml.load(contents), expectedType); + } } diff --git a/modules/swagger-parser/src/main/java/io/swagger/parser/util/SwaggerDeserializer.java b/modules/swagger-parser/src/main/java/io/swagger/parser/util/SwaggerDeserializer.java index df24f9119c..44034ef68f 100644 --- a/modules/swagger-parser/src/main/java/io/swagger/parser/util/SwaggerDeserializer.java +++ b/modules/swagger-parser/src/main/java/io/swagger/parser/util/SwaggerDeserializer.java @@ -38,18 +38,17 @@ public SwaggerDeserializationResult deserialize(JsonNode rootNode) { SwaggerDeserializationResult result = new SwaggerDeserializationResult(); ParseResult rootParse = new ParseResult(); - if(rootNode != null) { - Swagger swagger = parseRoot(rootNode, rootParse); - result.setSwagger(swagger); - result.setMessages(rootParse.getMessages()); - } + Swagger swagger = parseRoot(rootNode, rootParse); + result.setSwagger(swagger); + result.setMessages(rootParse.getMessages()); + return result; } public Swagger parseRoot(JsonNode node, ParseResult result) { String location = ""; Swagger swagger = new Swagger(); - if (node.getNodeType().equals(JsonNodeType.OBJECT)) { + if (node != null && node.getNodeType().equals(JsonNodeType.OBJECT)) { ObjectNode on = (ObjectNode)node; Iterator it = null; From 25b1240aa12e47d6205c63796c8b2ed6eb7450bf Mon Sep 17 00:00:00 2001 From: gracekarina Date: Fri, 28 Dec 2018 21:41:49 -0500 Subject: [PATCH 4/4] adding a better pattern --- .../main/java/io/swagger/parser/util/DeserializationUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/swagger-parser/src/main/java/io/swagger/parser/util/DeserializationUtils.java b/modules/swagger-parser/src/main/java/io/swagger/parser/util/DeserializationUtils.java index afcf0d0115..9f49cf4c03 100644 --- a/modules/swagger-parser/src/main/java/io/swagger/parser/util/DeserializationUtils.java +++ b/modules/swagger-parser/src/main/java/io/swagger/parser/util/DeserializationUtils.java @@ -64,7 +64,7 @@ private static boolean isJson(String contents) { } public static JsonNode readYamlTree(String contents) { - String pattern = "&"; + String pattern = "[>|&!%`@,.*]"; Pattern r = Pattern.compile(pattern); Matcher m = r.matcher(contents); org.yaml.snakeyaml.Yaml yaml = new org.yaml.snakeyaml.Yaml(new SafeConstructor());