From 3a7910da5a71e4dd6d7e59808582840c8e4bc25f Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Thu, 5 Mar 2020 14:55:40 +0000 Subject: [PATCH] Introduce (un-)marshaling functions as a concept and instantiate it with the functions in `encoding/json`. --- ql/src/semmle/go/Concepts.qll | 80 +++++++++++++++++++ ql/src/semmle/go/frameworks/Stdlib.qll | 25 +++++- .../go/security/StringBreakCustomizations.qll | 9 +-- 3 files changed, 106 insertions(+), 8 deletions(-) diff --git a/ql/src/semmle/go/Concepts.qll b/ql/src/semmle/go/Concepts.qll index 03950b66654..69306a1a724 100644 --- a/ql/src/semmle/go/Concepts.qll +++ b/ql/src/semmle/go/Concepts.qll @@ -589,3 +589,83 @@ module LoggerCall { abstract DataFlow::Node getAMessageComponent(); } } + +/** + * A function that encodes data into a binary or textual format. + * + * Extend this class to refine existing API models. If you want to model new APIs, + * extend `MarshalingFunction::Range` instead. + */ +class MarshalingFunction extends Function { + MarshalingFunction::Range self; + + MarshalingFunction() { this = self } + + /** Gets an input that is encoded by this function. */ + DataFlow::FunctionInput getAnInput() { result = self.getAnInput() } + + /** Gets the output that contains the encoded data produced by this function. */ + DataFlow::FunctionOutput getOutput() { result = self.getOutput() } + + /** Gets an identifier for the format this function encodes into, such as "JSON". */ + string getFormat() { result = self.getFormat() } +} + +module MarshalingFunction { + /** + * A function that encodes data into a binary or textual format. + * + * Extend this class to model new APIs. If you want to refine existing API models, + * extend `MarshalingFunction` instead. + */ + abstract class Range extends Function { + /** Gets an input that is encoded by this function. */ + abstract DataFlow::FunctionInput getAnInput(); + + /** Gets the output that contains the encoded data produced by this function. */ + abstract DataFlow::FunctionOutput getOutput(); + + /** Gets an identifier for the format this function encodes into, such as "JSON". */ + abstract string getFormat(); + } +} + +/** + * A function that decodes data from a binary or textual format. + * + * Extend this class to refine existing API models. If you want to model new APIs, + * extend `UnmarshalingFunction::Range` instead. + */ +class UnmarshalingFunction extends Function { + UnmarshalingFunction::Range self; + + UnmarshalingFunction() { this = self } + + /** Gets an input that is decoded by this function. */ + DataFlow::FunctionInput getAnInput() { result = self.getAnInput() } + + /** Gets the output that contains the decoded data produced by this function. */ + DataFlow::FunctionOutput getOutput() { result = self.getOutput() } + + /** Gets an identifier for the format this function decodes from, such as "JSON". */ + string getFormat() { result = self.getFormat() } +} + +module UnmarshalingFunction { + /** + * A function that decodes data from a binary or textual format. + * + * Extend this class to model new APIs. If you want to refine existing API models, + * extend `UnmarshalingFunction` instead. + */ + abstract class Range extends Function { + /** Gets an input that is decoded by this function. */ + abstract DataFlow::FunctionInput getAnInput(); + + /** Gets the output that contains the decoded data produced by this function. */ + abstract DataFlow::FunctionOutput getOutput(); + + /** Gets an identifier for the format this function decodes from, such as "JSON". */ + abstract string getFormat(); + } +} diff --git a/ql/src/semmle/go/frameworks/Stdlib.qll b/ql/src/semmle/go/frameworks/Stdlib.qll index 63d66ad5778..48b4d43669f 100644 --- a/ql/src/semmle/go/frameworks/Stdlib.qll +++ b/ql/src/semmle/go/frameworks/Stdlib.qll @@ -519,16 +519,35 @@ module Log { /** Provides models of some functions in the `encoding/json` package. */ module EncodingJson { - private class MarshalFunction extends TaintTracking::FunctionModel { + private class MarshalFunction extends TaintTracking::FunctionModel, MarshalingFunction::Range { MarshalFunction() { this.hasQualifiedName("encoding/json", "Marshal") or this.hasQualifiedName("encoding/json", "MarshalIndent") } override predicate hasTaintFlow(DataFlow::FunctionInput inp, DataFlow::FunctionOutput outp) { - inp.isParameter(0) and - outp.isResult(0) + inp = getAnInput() and outp = getOutput() } + + override DataFlow::FunctionInput getAnInput() { result.isParameter(0) } + + override DataFlow::FunctionOutput getOutput() { result.isResult(0) } + + override string getFormat() { result = "JSON" } + } + + private class UnmarshalFunction extends TaintTracking::FunctionModel, UnmarshalingFunction::Range { + UnmarshalFunction() { this.hasQualifiedName("encoding/json", "Unmarshal") } + + override predicate hasTaintFlow(DataFlow::FunctionInput inp, DataFlow::FunctionOutput outp) { + inp = getAnInput() and outp = getOutput() + } + + override DataFlow::FunctionInput getAnInput() { result.isParameter(0) } + + override DataFlow::FunctionOutput getOutput() { result.isParameter(1) } + + override string getFormat() { result = "JSON" } } } diff --git a/ql/src/semmle/go/security/StringBreakCustomizations.qll b/ql/src/semmle/go/security/StringBreakCustomizations.qll index 0e58ee15efd..b2330f59d18 100644 --- a/ql/src/semmle/go/security/StringBreakCustomizations.qll +++ b/ql/src/semmle/go/security/StringBreakCustomizations.qll @@ -48,9 +48,8 @@ module StringBreak { /** A call to `json.Marshal`, considered as a taint source for unsafe quoting. */ class JsonMarshalAsSource extends Source { JsonMarshalAsSource() { - exists(Function jsonMarshal | jsonMarshal.hasQualifiedName("encoding/json", "Marshal") | - // we are only interested in the first result (the second result is an error) - this = DataFlow::extractTupleElement(jsonMarshal.getACall(), 0) + exists(MarshalingFunction jsonMarshal | jsonMarshal.getFormat() = "JSON" | + this = jsonMarshal.getOutput().getNode(jsonMarshal.getACall()) ) } } @@ -72,8 +71,8 @@ module StringBreak { /** A call to `json.Unmarshal`, considered as a sanitizer for unsafe quoting. */ class UnmarshalSanitizer extends Sanitizer { UnmarshalSanitizer() { - exists(Function jsonUnmarshal | jsonUnmarshal.hasQualifiedName("encoding/json", "Unmarshal") | - this = jsonUnmarshal.getACall() + exists(UnmarshalingFunction jsonUnmarshal | jsonUnmarshal.getFormat() = "JSON" | + this = jsonUnmarshal.getOutput().getNode(jsonUnmarshal.getACall()) ) } }