Skip to content

Go: Make DataFlowType a singleton #11697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 15, 2022
Merged

Go: Make DataFlowType a singleton #11697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50414cc
Make DataFlowType a singleton
owen-mc Dec 14, 2022
d18179c
Add QLDoc for `toString` on `DataFlowType`
owen-mc Dec 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,21 +199,17 @@ predicate expectsContent(Node n, ContentSet c) {
}

/** Gets the type of `n` used for type pruning. */
DataFlowType getNodeType(Node n) {
result = n.getType()
or
result = FlowSummaryImpl::Private::summaryNodeType(n)
}
DataFlowType getNodeType(Node n) { result = TTodoDataFlowType() and exists(n) }

/** Gets a string representation of a type returned by `getNodeType()`. */
string ppReprType(Type t) { result = t.toString() }
string ppReprType(DataFlowType t) { none() }

/**
* Holds if `t1` and `t2` are compatible, that is, whether data can flow from
* a node of type `t1` to a node of type `t2`.
*/
pragma[inline]
predicate compatibleTypes(Type t1, Type t2) {
predicate compatibleTypes(DataFlowType t1, DataFlowType t2) {
any() // stub implementation
}

Expand All @@ -227,7 +223,14 @@ class CastNode extends ExprNode {

class DataFlowExpr = Expr;

class DataFlowType = Type;
private newtype TDataFlowType =
TTodoDataFlowType() or
TTodoDataFlowType2() // Add a dummy value to prevent bad functionality-induced joins arising from a type of size 1.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I note only some other languages do this -- might as well avoid the hack if not necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those languages did need it though. I assume it wouldn't be hard to remove our hack when the other hacks are removed, if it isn't needed any more. Are you suggesting that I omit the second branch, run a performance check to see if we can get by without it, and if so then omit it and hope that we don't hit the performance problem in future?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically yes, though I wouldn't object to the flow merge as-is (after the requested double-check below), then experimentally remove the hack from all languages.

Suggest also querying on slack whether the comment about singleton types still applies or has perhaps been fixed since.


class DataFlowType extends TDataFlowType {
/** Gets a textual representation of this element. */
string toString() { result = "" }
}

class DataFlowLocation = Location;

Expand Down
8 changes: 4 additions & 4 deletions go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ private newtype TContent =
*/
class Content extends TContent {
/** Gets the type of the contained data for the purpose of type pruning. */
DataFlowType getType() { result instanceof EmptyInterfaceType }
DataFlowType getType() { any() }

/** Gets a textual representation of this element. */
abstract string toString();
Expand Down Expand Up @@ -177,7 +177,7 @@ class FieldContent extends Content, TFieldContent {
/** Gets the field associated with this `FieldContent`. */
Field getField() { result = f }

override DataFlowType getType() { result = f.getType() }
override DataFlowType getType() { any() }

override string toString() { result = f.toString() }

Expand Down Expand Up @@ -205,7 +205,7 @@ class PointerContent extends Content, TPointerContent {
/** Gets the pointer type that containers with this content must have. */
PointerType getPointerType() { result = t }

override DataFlowType getType() { result = t.getBaseType() }
override DataFlowType getType() { any() }

override string toString() { result = "pointer" }
}
Expand All @@ -228,7 +228,7 @@ class SyntheticFieldContent extends Content, TSyntheticFieldContent {
/** Gets the field associated with this `SyntheticFieldContent`. */
SyntheticField getField() { result = s }

override DataFlowType getType() { result = s.getType() }
override DataFlowType getType() { any() }

override string toString() { result = s.toString() }
}
Expand Down
4 changes: 1 addition & 3 deletions go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImplSpecific.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ DataFlowCall summaryDataFlowCall(Node receiver) {
DataFlowType getContentType(Content c) { result = c.getType() }

/** Gets the return type of kind `rk` for callable `c`. */
DataFlowType getReturnType(SummarizedCallable c, ReturnKind rk) {
result = c.getType().getResultType(rk.getIndex())
}
DataFlowType getReturnType(SummarizedCallable c, ReturnKind rk) { any() }

/**
* Gets the type of the `i`th parameter in a synthesized call that targets a
Expand Down
76 changes: 38 additions & 38 deletions go/ql/test/experimental/CWE-090/LDAPInjection.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,53 +1,53 @@
edges
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:59:3:59:11 | untrusted |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:61:3:61:51 | ...+... |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:62:3:62:33 | slice literal |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:62:24:62:32 | untrusted : string |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:66:3:66:11 | untrusted |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:68:3:68:51 | ...+... |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:69:3:69:33 | slice literal |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:69:24:69:32 | untrusted : string |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:73:3:73:11 | untrusted |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:75:3:75:51 | ...+... |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:76:3:76:33 | slice literal |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:76:24:76:32 | untrusted : string |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:80:22:80:30 | untrusted |
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:81:25:81:33 | untrusted |
| LDAPInjection.go:62:3:62:33 | slice literal [array] : string | LDAPInjection.go:62:3:62:33 | slice literal |
| LDAPInjection.go:62:24:62:32 | untrusted : string | LDAPInjection.go:62:3:62:33 | slice literal [array] : string |
| LDAPInjection.go:69:3:69:33 | slice literal [array] : string | LDAPInjection.go:69:3:69:33 | slice literal |
| LDAPInjection.go:69:24:69:32 | untrusted : string | LDAPInjection.go:69:3:69:33 | slice literal [array] : string |
| LDAPInjection.go:76:3:76:33 | slice literal [array] : string | LDAPInjection.go:76:3:76:33 | slice literal |
| LDAPInjection.go:76:24:76:32 | untrusted : string | LDAPInjection.go:76:3:76:33 | slice literal [array] : string |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:59:3:59:11 | untrusted |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:61:3:61:51 | ...+... |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:3:62:33 | slice literal |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:24:62:32 | untrusted |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:66:3:66:11 | untrusted |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:68:3:68:51 | ...+... |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:3:69:33 | slice literal |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:24:69:32 | untrusted |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:73:3:73:11 | untrusted |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:75:3:75:51 | ...+... |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:3:76:33 | slice literal |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:24:76:32 | untrusted |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:80:22:80:30 | untrusted |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:81:25:81:33 | untrusted |
| LDAPInjection.go:62:3:62:33 | slice literal [array] | LDAPInjection.go:62:3:62:33 | slice literal |
| LDAPInjection.go:62:24:62:32 | untrusted | LDAPInjection.go:62:3:62:33 | slice literal [array] |
| LDAPInjection.go:69:3:69:33 | slice literal [array] | LDAPInjection.go:69:3:69:33 | slice literal |
| LDAPInjection.go:69:24:69:32 | untrusted | LDAPInjection.go:69:3:69:33 | slice literal [array] |
| LDAPInjection.go:76:3:76:33 | slice literal [array] | LDAPInjection.go:76:3:76:33 | slice literal |
| LDAPInjection.go:76:24:76:32 | untrusted | LDAPInjection.go:76:3:76:33 | slice literal [array] |
nodes
| LDAPInjection.go:57:15:57:29 | call to UserAgent : string | semmle.label | call to UserAgent : string |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | semmle.label | call to UserAgent |
| LDAPInjection.go:59:3:59:11 | untrusted | semmle.label | untrusted |
| LDAPInjection.go:61:3:61:51 | ...+... | semmle.label | ...+... |
| LDAPInjection.go:62:3:62:33 | slice literal | semmle.label | slice literal |
| LDAPInjection.go:62:3:62:33 | slice literal [array] : string | semmle.label | slice literal [array] : string |
| LDAPInjection.go:62:24:62:32 | untrusted : string | semmle.label | untrusted : string |
| LDAPInjection.go:62:3:62:33 | slice literal [array] | semmle.label | slice literal [array] |
| LDAPInjection.go:62:24:62:32 | untrusted | semmle.label | untrusted |
| LDAPInjection.go:66:3:66:11 | untrusted | semmle.label | untrusted |
| LDAPInjection.go:68:3:68:51 | ...+... | semmle.label | ...+... |
| LDAPInjection.go:69:3:69:33 | slice literal | semmle.label | slice literal |
| LDAPInjection.go:69:3:69:33 | slice literal [array] : string | semmle.label | slice literal [array] : string |
| LDAPInjection.go:69:24:69:32 | untrusted : string | semmle.label | untrusted : string |
| LDAPInjection.go:69:3:69:33 | slice literal [array] | semmle.label | slice literal [array] |
| LDAPInjection.go:69:24:69:32 | untrusted | semmle.label | untrusted |
| LDAPInjection.go:73:3:73:11 | untrusted | semmle.label | untrusted |
| LDAPInjection.go:75:3:75:51 | ...+... | semmle.label | ...+... |
| LDAPInjection.go:76:3:76:33 | slice literal | semmle.label | slice literal |
| LDAPInjection.go:76:3:76:33 | slice literal [array] : string | semmle.label | slice literal [array] : string |
| LDAPInjection.go:76:24:76:32 | untrusted : string | semmle.label | untrusted : string |
| LDAPInjection.go:76:3:76:33 | slice literal [array] | semmle.label | slice literal [array] |
| LDAPInjection.go:76:24:76:32 | untrusted | semmle.label | untrusted |
| LDAPInjection.go:80:22:80:30 | untrusted | semmle.label | untrusted |
| LDAPInjection.go:81:25:81:33 | untrusted | semmle.label | untrusted |
subpaths
#select
| LDAPInjection.go:59:3:59:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:59:3:59:11 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:61:3:61:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:61:3:61:51 | ...+... | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:62:3:62:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:62:3:62:33 | slice literal | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:66:3:66:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:66:3:66:11 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:68:3:68:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:68:3:68:51 | ...+... | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:69:3:69:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:69:3:69:33 | slice literal | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:73:3:73:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:73:3:73:11 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:75:3:75:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:75:3:75:51 | ...+... | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:76:3:76:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:76:3:76:33 | slice literal | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:80:22:80:30 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:80:22:80:30 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:81:25:81:33 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:81:25:81:33 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:59:3:59:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:59:3:59:11 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:61:3:61:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:61:3:61:51 | ...+... | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:62:3:62:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:3:62:33 | slice literal | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:66:3:66:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:66:3:66:11 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:68:3:68:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:68:3:68:51 | ...+... | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:69:3:69:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:3:69:33 | slice literal | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:73:3:73:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:73:3:73:11 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:75:3:75:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:75:3:75:51 | ...+... | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:76:3:76:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:3:76:33 | slice literal | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:80:22:80:30 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:80:22:80:30 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
| LDAPInjection.go:81:25:81:33 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:81:25:81:33 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
Loading