From 35b4c438ff09bd431858bc166121007bae933cca Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Wed, 7 Jun 2023 14:12:20 +0200 Subject: [PATCH 1/2] Fix Gson's JsonArray.add models When the type of the argument isn't JsonElement, the summary must be taint flow instead of value flow --- java/ql/lib/ext/com.google.gson.model.yml | 7 +++++- .../library-tests/frameworks/gson/Test.java | 24 +++++++++---------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/java/ql/lib/ext/com.google.gson.model.yml b/java/ql/lib/ext/com.google.gson.model.yml index 96f5355b2dc..73e14cf7cc8 100644 --- a/java/ql/lib/ext/com.google.gson.model.yml +++ b/java/ql/lib/ext/com.google.gson.model.yml @@ -26,7 +26,12 @@ extensions: - ["com.google.gson", "JsonElement", True, "getAsJsonPrimitive", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"] - ["com.google.gson", "JsonElement", True, "getAsString", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"] - ["com.google.gson", "JsonElement", True, "toString", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"] - - ["com.google.gson", "JsonArray", True, "add", "", "", "Argument[0]", "Argument[this].Element", "value", "manual"] + - ["com.google.gson", "JsonArray", True, "add", "(Boolean)", "", "Argument[0]", "Argument[this].Element", "taint", "manual"] + - ["com.google.gson", "JsonArray", True, "add", "(Character)", "", "Argument[0]", "Argument[this].Element", "taint", "manual"] + - ["com.google.gson", "JsonArray", True, "add", "(JsonElement)", "", "Argument[0]", "Argument[this].Element", "value", "manual"] + - ["com.google.gson", "JsonArray", True, "add", "(Number)", "", "Argument[0]", "Argument[this].Element", "taint", "manual"] + - ["com.google.gson", "JsonArray", True, "add", "(String)", "", "Argument[0]", "Argument[this].Element", "taint", "manual"] + - ["com.google.gson", "JsonArray", True, "add", "(JsonArray)", "", "Argument[0].Element", "Argument[this].Element", "value", "manual"] - ["com.google.gson", "JsonArray", True, "asList", "", "", "Argument[this].Element", "ReturnValue.Element", "value", "manual"] - ["com.google.gson", "JsonArray", True, "get", "", "", "Argument[this].Element", "ReturnValue", "value", "manual"] - ["com.google.gson", "JsonArray", True, "set", "", "", "Argument[1]", "Argument[this].Element", "value", "manual"] diff --git a/java/ql/test/library-tests/frameworks/gson/Test.java b/java/ql/test/library-tests/frameworks/gson/Test.java index eb3e1e526f0..00811587e8b 100644 --- a/java/ql/test/library-tests/frameworks/gson/Test.java +++ b/java/ql/test/library-tests/frameworks/gson/Test.java @@ -25,7 +25,7 @@ public class Test { K getMapKeyDefault(Map.Entry container) { return container.getKey(); } JsonElement getMapValueDefault(JsonObject container) { return container.get(null); } V getMapValueDefault(Map.Entry container) { return container.getValue(); } - JsonArray newWithElementDefault(String element) { JsonArray a = new JsonArray(); a.add(element); return a; } + JsonArray newWithElementDefault(JsonElement element) { JsonArray a = new JsonArray(); a.add(element); return a; } JsonObject newWithMapKeyDefault(String key) { JsonObject o = new JsonObject(); o.add(key, (JsonElement) null); return o; } JsonObject newWithMapValueDefault(JsonElement element) { JsonObject o = new JsonObject(); o.add(null, element); return o; } Object source() { return null; } @@ -232,51 +232,51 @@ public class Test { sink(out); // $ hasTaintFlow } { - // "com.google.gson;JsonArray;true;add;;;Argument[0];Argument[this].Element;value;manual" + // "com.google.gson;JsonArray;true;add;(Boolean);;Argument[0];Argument[this].Element;taint;manual" JsonArray out = null; Boolean in = (Boolean)source(); out.add(in); - sink(getElement(out)); // $ hasValueFlow + sink(getElement(out)); // $ hasTaintFlow } { - // "com.google.gson;JsonArray;true;add;;;Argument[0];Argument[this].Element;value;manual" + // "com.google.gson;JsonArray;true;add;(Character);;Argument[0];Argument[this].Element;taint;manual" JsonArray out = null; Character in = (Character)source(); out.add(in); - sink(getElement(out)); // $ hasValueFlow + sink(getElement(out)); // $ hasTaintFlow } { - // "com.google.gson;JsonArray;true;add;;;Argument[0];Argument[this].Element;value;manual" + // "com.google.gson;JsonArray;true;add;(JsonElement);;Argument[0];Argument[this].Element;value;manual" JsonArray out = null; JsonElement in = (JsonElement)source(); out.add(in); sink(getElement(out)); // $ hasValueFlow } { - // "com.google.gson;JsonArray;true;add;;;Argument[0];Argument[this].Element;value;manual" + // "com.google.gson;JsonArray;true;add;(Number);;Argument[0];Argument[this].Element;taint;manual" JsonArray out = null; Number in = (Number)source(); out.add(in); - sink(getElement(out)); // $ hasValueFlow + sink(getElement(out)); // $ hasTaintFlow } { - // "com.google.gson;JsonArray;true;add;;;Argument[0];Argument[this].Element;value;manual" + // "com.google.gson;JsonArray;true;add;(JsonArray);;Argument[0].Element;Argument[this].Element;value;manual" JsonArray out = null; - String in = (String)source(); + JsonElement in = (JsonElement)source(); out.add(in); sink(getElement(out)); // $ hasValueFlow } { // "com.google.gson;JsonArray;true;asList;;;Argument[this].Element;ReturnValue.Element;value;manual" List out = null; - JsonArray in = (JsonArray)newWithElementDefault((String) source()); + JsonArray in = (JsonArray)newWithElementDefault((JsonElement) source()); out = in.asList(); sink(getElement(out)); // $ hasValueFlow } { // "com.google.gson;JsonArray;true;get;;;Argument[this].Element;ReturnValue;value;manual" JsonElement out = null; - JsonArray in = (JsonArray)newWithElementDefault((String) source()); + JsonArray in = (JsonArray)newWithElementDefault((JsonElement) source()); out = in.get(0); sink(out); // $ hasValueFlow } From c0135673fac4bf62bef7da411eef2b10ad51d696 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Wed, 7 Jun 2023 16:18:32 +0200 Subject: [PATCH 2/2] Fix JsonArray.addAll model Properly test JsonArray.add(String) and JsonArray.addAll(JsonArray) as well --- java/ql/lib/ext/com.google.gson.model.yml | 2 +- .../test/library-tests/frameworks/gson/Test.java | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/java/ql/lib/ext/com.google.gson.model.yml b/java/ql/lib/ext/com.google.gson.model.yml index 73e14cf7cc8..7b41b57083a 100644 --- a/java/ql/lib/ext/com.google.gson.model.yml +++ b/java/ql/lib/ext/com.google.gson.model.yml @@ -31,7 +31,7 @@ extensions: - ["com.google.gson", "JsonArray", True, "add", "(JsonElement)", "", "Argument[0]", "Argument[this].Element", "value", "manual"] - ["com.google.gson", "JsonArray", True, "add", "(Number)", "", "Argument[0]", "Argument[this].Element", "taint", "manual"] - ["com.google.gson", "JsonArray", True, "add", "(String)", "", "Argument[0]", "Argument[this].Element", "taint", "manual"] - - ["com.google.gson", "JsonArray", True, "add", "(JsonArray)", "", "Argument[0].Element", "Argument[this].Element", "value", "manual"] + - ["com.google.gson", "JsonArray", True, "addAll", "(JsonArray)", "", "Argument[0].Element", "Argument[this].Element", "value", "manual"] - ["com.google.gson", "JsonArray", True, "asList", "", "", "Argument[this].Element", "ReturnValue.Element", "value", "manual"] - ["com.google.gson", "JsonArray", True, "get", "", "", "Argument[this].Element", "ReturnValue", "value", "manual"] - ["com.google.gson", "JsonArray", True, "set", "", "", "Argument[1]", "Argument[this].Element", "value", "manual"] diff --git a/java/ql/test/library-tests/frameworks/gson/Test.java b/java/ql/test/library-tests/frameworks/gson/Test.java index 00811587e8b..6fa1fd2a1e5 100644 --- a/java/ql/test/library-tests/frameworks/gson/Test.java +++ b/java/ql/test/library-tests/frameworks/gson/Test.java @@ -260,23 +260,30 @@ public class Test { sink(getElement(out)); // $ hasTaintFlow } { - // "com.google.gson;JsonArray;true;add;(JsonArray);;Argument[0].Element;Argument[this].Element;value;manual" + // "com.google.gson;JsonArray;true;add;(String);;Argument[0];Argument[this].Element;taint;manual" JsonArray out = null; - JsonElement in = (JsonElement)source(); + String in = (String)source(); out.add(in); + sink(getElement(out)); // $ hasTaintFlow + } + { + // "com.google.gson;JsonArray;true;addAll;(JsonArray);;Argument[0].Element;Argument[this].Element;value;manual" + JsonArray out = null; + JsonArray in = newWithElementDefault((JsonElement) source()); + out.addAll(in); sink(getElement(out)); // $ hasValueFlow } { // "com.google.gson;JsonArray;true;asList;;;Argument[this].Element;ReturnValue.Element;value;manual" List out = null; - JsonArray in = (JsonArray)newWithElementDefault((JsonElement) source()); + JsonArray in = newWithElementDefault((JsonElement) source()); out = in.asList(); sink(getElement(out)); // $ hasValueFlow } { // "com.google.gson;JsonArray;true;get;;;Argument[this].Element;ReturnValue;value;manual" JsonElement out = null; - JsonArray in = (JsonArray)newWithElementDefault((JsonElement) source()); + JsonArray in = newWithElementDefault((JsonElement) source()); out = in.get(0); sink(out); // $ hasValueFlow }