C++: IR field flow #3118

MathiasVP · 2020-03-24T21:35:52Z

This PR takes the the first stab at implementing field flow in the IR.

This is still WIP, but any comments is appreciated. One thing I remember is @jbj mentioning that DefinitionByReferenceNode couldn't be made a PostUpdateNode because there's not a corresponding pre update node to pick, but it seemed like this was the most obvious way to model partial flow to the this parameter in setters. And for now #2921 only reports a couple of problems with nodes missing locations, which shouldn't be a problem to fix.

The failing testcases are from the Semmle/code, where we flag a new bad case for cpp/user-controlled-bypass. When we are happy with the state of this PR I will make an internal PR and update that test.

…ll arguments need a PostUpdateNode). Also generalized the added flow rule in simpleLocalFlowStep since there isn't always a ChiInstruction - for instance of it's a write to a struct that only has a single field.

…than helpful

jbj · 2020-03-26T11:03:57Z

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

@@ -321,6 +354,13 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) { simpleLocalFlowStep(nodeFr
 */
 predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
  simpleInstructionLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asInstruction())
+  or
+  exists(LoadInstruction load |
+    // TODO: These can probably be getSourceValue() after #3112 is merged


It's now merged (that's the line I meant to comment on a minute ago).

Sadly my comment turned out to be incorrect. The flow in the following program is not captured by only following exact Chi operands:

void sink(int *o); struct B { int *c; void set(int *c) { this->c = c; } }; void f7(B *b) { b->set(new int); } void f8() { B *b = new B(); f7(b); sink(b->c); // flow }

since the load on b->c only is a total overlap.

Then you'll want to merge #3097 and use isResultConflated here.

MathiasVP · 2020-03-26T11:10:55Z

Here's a quick rundown of the changes in the output:

AuthenticationBypass.qlref in Semmle/code has 1 new true positive
ql/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_ir.ql has 3 new true positives and 2 new false positives (the 2 false positives were also present in the old field flow library according to the comments)
The changes to ql/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.ql and ql/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.ql looks like the expected output given that it's tracking taint.

jbj · 2020-03-26T12:27:54Z

ql/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_ir.ql has 3 new true positives and 2 new false positives (the 2 false positives were also present in the old field flow library according to the comments)

And those two false positives are caused by limitations of the shared field-flow library rather than the C++ instantiation of it. They'll be present in the other languages too.

MathiasVP · 2020-03-27T09:48:31Z

Testing locally on Wireshark/Wireshark revealed something that looks like a bad join order in DataFlowImpl::parameterThroughFlowCand:

[2020-03-27 10:13:23] (3432s) Tuple counts for DataFlowImpl::parameterThroughFlowCand#ff#antijoin_rhs:
                      2981323763 ~0%     {3} r1 = JOIN DataFlowImpl::parameterThroughFlowCand#ff#shared AS L WITH DataFlowUtil::InstructionNode::getEnclosingCallable_dispred#ff_10#join_rhs AS R ON FIRST 1 OUTPUT L.<2>, L.<1>, R.<1>
                      2968445848 ~2%     {4} r2 = JOIN r1 WITH DataFlowImplCommon::Cached::TParamUpdate#2#ff_10#join_rhs AS R ON FIRST 1 OUTPUT r1.<2>, R.<1>, r1.<1>, r1.<0>
                      832264     ~0%     {3} r3 = JOIN r2 WITH project#DataFlowUtil::ParameterNode::isParameterOf_dispred#2#fff AS R ON FIRST 2 OUTPUT r2.<2>, r2.<3>, r2.<0>
                                         return r3

I'm looking into this now.

Edit: Adding pragma[nomagic] to returnFlowCallableCand fixes the problem.

MathiasVP · 2020-03-28T11:31:00Z

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

@@ -219,7 +219,7 @@ abstract class PostUpdateNode extends InstructionNode {
 abstract private class PartialDefinitionNode extends PostUpdateNode, TInstructionNode {
  final Instruction getInstructionOrChi() {
    exists(ChiInstruction chi |
-      // TODO: This should be a non-conflated ChiInstruction once #3123 is merged
+      not chi.isResultConflated() and


With this change we lose flow in the following case:

void sink(int *); struct B { int *i; }; void f7(B* b) { b->i = source(); } void f8() { B* b = new B; f7(b); sink(b->i); // flow [not detected] }

Because the BufferMayWriteSideEffect for b in f7(b) is melded into all aliased memory:

# 15| r15_1(glval<unknown>) = FunctionAddress[f7] : # 15| r15_2(glval<B *>) = VariableAddress[b] : # 15| r15_3(B *) = Load : &:r15_2, m14_8 # 15| v15_4(void) = Call : func:r15_1, 0:r15_3 # 15| m15_5(unknown) = ^CallSideEffect : ~m14_6 # 15| m15_6(unknown) = Chi : total:m14_6, partial:m15_5 # 15| v15_7(void) = ^BufferReadSideEffect[0] : &:r15_3, ~m15_6 # 15| m15_8(unknown) = ^BufferMayWriteSideEffect[0] : &:r15_3 # 15| m15_9(unknown) = Chi : total:m15_6, partial:m15_8 # 16| r16_1(glval<unknown>) = FunctionAddress[sink] : # 16| r16_2(glval<B *>) = VariableAddress[b] : # 16| r16_3(B *) = Load : &:r16_2, m14_8 # 16| r16_4(glval<int *>) = FieldAddress[i] : r16_3 # 16| r16_5(int *) = Load : &:r16_4, ~m15_9 # 16| v16_6(void) = Call : func:r16_1, 0:r16_5 # 16| m16_7(unknown) = ^CallSideEffect : ~m15_9

I'm not totally sure how to recover this flow without accepting flow through Chi instructions that update all aliased memory.

Interestingly, the flow is reported in the following modified program:

void sink(int *); struct B { int *i; }; void f7(B* b) { b->i = source(); } void f8(B* b) { f7(b); sink(b->i); // flow }

since the Chi following the BufferMayWriteSideEffect does not update all aliased memory:

# 15| r15_1(glval<unknown>) = FunctionAddress[f7] : # 15| r15_2(glval<B *>) = VariableAddress[b] : # 15| r15_3(B *) = Load : &:r15_2, m12_7 # 15| v15_4(void) = Call : func:r15_1, 0:r15_3 # 15| m15_5(unknown) = ^CallSideEffect : ~m12_4 # 15| m15_6(unknown) = Chi : total:m12_4, partial:m15_5 # 15| v15_7(void) = ^BufferReadSideEffect[0] : &:r15_3, ~m12_9 # 15| m15_8(unknown) = ^BufferMayWriteSideEffect[0] : &:r15_3 # 15| m15_9(int *) = Chi : total:m12_9, partial:m15_8 # 16| r16_1(glval<unknown>) = FunctionAddress[sink] : # 16| r16_2(glval<B *>) = VariableAddress[b] : # 16| r16_3(B *) = Load : &:r16_2, m12_7 # 16| r16_4(glval<int *>) = FieldAddress[i] : r16_3 # 16| r16_5(int *) = Load : &:r16_4, ~m15_9 # 16| v16_6(void) = Call : func:r16_1, 0:r16_5 # 16| m16_7(unknown) = ^CallSideEffect : ~m15_6

Is that a flaw in our support for allocations (#2797)? I would think that new B and a B * parameter should be treated the same by the current (unsound) alias analysis.

Indeed, a fresh allocation certainly shouldn't alias more stuff than a B* parameter. I'll look at this issue later today.

Merging in #3171 fixes the problem.

MathiasVP · 2020-03-28T23:47:59Z

CPP-difference run: https://jenkins.internal.semmle.com/job/Changes/job/CPP-Differences/996

jbj · 2020-03-30T07:17:39Z

CPP-difference run: https://jenkins.internal.semmle.com/job/Changes/job/CPP-Differences/996

Most of the differences on openjdk come from #3123. The two changes for cpp/signed-overflow-check are also from a different PR, but I forget which.

MathiasVP · 2020-03-30T07:24:11Z

Most of the differences on openjdk come from #3123. The two changes for cpp/signed-overflow-check are also from a different PR, but I forget which.

Oh yeah, good point. I should have chosen my baseline commit more carefully. I'll create a new one that compares against master

jbj · 2020-03-30T07:27:01Z

I don't think you want to compare against master since some of the unrelated changes come from unmerged PRs. You want the baseline to be a branch with all the PRs that you've included in this PR (on top of the merge-base to master).

MathiasVP · 2020-03-30T07:33:53Z

I don't think you want to compare against master since some of the unrelated changes come from unmerged PRs. You want the baseline to be a branch with all the PRs that you've included in this PR (on top of the merge-base to master).

I don't think I have included any unmerged PRs in here yet? So far I've only merged from master. So #3123 isn't actually included in this PR. Only the part in #3097.

MathiasVP · 2020-03-31T12:56:07Z

Here's a more honest CPP-differences run: https://jenkins.internal.semmle.com/job/Changes/job/CPP-Differences/1002. I've looked at the results for linux and openjdk, and they all seem like results we want to flag.

MathiasVP · 2020-04-08T06:49:24Z

A fresh CPP-differences run: https://jenkins.internal.semmle.com/job/Changes/job/CPP-Differences/1020/

jbj · 2020-04-08T07:10:05Z

A fresh CPP-differences run: https://jenkins.internal.semmle.com/job/Changes/job/CPP-Differences/1020/

It's not all that fresh -- it seems to be based on an old master from 2 April. There's clearly some performance work to be done. Be sure to merge from the latest master, including path explanations, before investigating performance.

MathiasVP · 2020-04-08T07:13:45Z

It's not all that fresh -- it seems to be based on an old master from 2 April. There's clearly some performance work to be done. Be sure to merge from the latest master, including path explanations, before investigating performance.

Will do. Fresh was the wrong choice of word here. I meant fresh as in "a new one based on the changes made to this PR". It should be fine to use it for investigation of query results at least.

MathiasVP · 2020-04-10T10:13:03Z

The latest CPP-Differences revealed a performance regression due to the following predicate in DataFlowImplCommon.qll:

pragma[nomagic]
private predicate parameterValueFlow0(
  ParameterNode p, Node node, ContentOption contentIn, ContentOption contentOut
) {
  p = node and
  Cand::cand(p, _) and
  contentIn = TContentNone() and
  contentOut = TContentNone()
  or
  // local flow
  exists(Node mid |
    parameterValueFlow(p, mid, contentIn, contentOut) and
    LocalFlowBigStep::localFlowBigStep(mid, node)
  )
  or
  ...
}

with this PR we get the following tuple counts for iteration 5:

(2455s) Tuple counts for DataFlowImplCommon::Cached::FlowThrough::Final::parameterValueFlow0#ffff#cur_delta:
1579748   ~0%         {4} r1 = SCAN DataFlowImplCommon::Cached::FlowThrough::Final::parameterValueFlow#ffff#prev_delta AS I OUTPUT I.<1>, I.<0>, I.<2>, I.<3>
109572057 ~11994%     {4} r2 = JOIN r1 WITH DataFlowImplCommon::Cached::FlowThrough::LocalFlowBigStep::localFlowBigStep#ff AS R ON FIRST 1 OUTPUT R.<1>, r1.<1>, r1.<2>, r1.<3>
...
109577945 ~11984%     {4} r15 = r2 \/ r14
...
109580255 ~11974%     {4} r27 = r15 \/ r26
...
109580255 ~11974%     {4} r30 = r27 \/ r29
...
109580255 ~11974%     {4} r33 = r30 \/ r32
...
109580255 ~11974%     {4} r36 = r33 \/ r35
...
109580255 ~11974%     {4} r39 = r36 \/ r38
...
109580255 ~11974%     {4} r42 = r39 \/ r41
...
109580255 ~11974%     {4} r45 = r42 \/ r44
14493     ~4%         {4} r46 = r45 AND NOT DataFlowImplCommon::Cached::FlowThrough::Final::parameterValueFlow0#ffff#prev AS R(r45.<1>, r45.<0>, r45.<2>, r45.<3>)
14493     ~6%         {4} r47 = SCAN r46 OUTPUT r46.<1>, r46.<0>, r46.<2>, r46.<3>
                    return r47

where's on the current master (6e88b6d) we get:

(61s) Tuple counts for DataFlowImplCommon::Cached::FlowThrough::Final::parameterValueFlow0#ffff#cur_delta:
57061  ~0%     {4} r1 = SELECT DataFlowImplCommon::Cached::FlowThrough::Final::parameterValueFlow#ffff#prev_delta AS I ON I.<2> = I.<3>
57061  ~3%     {3} r2 = SCAN r1 OUTPUT r1.<1>, r1.<0>, r1.<2>
0      ~0%     {3} r3 = JOIN r2 WITH DataFlowImplCommon::Cached::FlowThrough::LocalFlowBigStep::localFlowBigStep#ff AS R ON FIRST 1 OUTPUT r2.<1>, r2.<2>, R.<1>
...
0      ~0%     {3} r9 = r3 \/ r8
...
0      ~0%     {3} r15 = r9 \/ r14
...
0      ~0%     {3} r20 = r15 \/ r19
...
0      ~0%     {3} r25 = r20 \/ r24
...
0      ~0%     {3} r28 = r25 \/ r27
...
0      ~0%     {3} r31 = r28 \/ r30
106958 ~0%     {4} r32 = SELECT DataFlowImplCommon::Cached::FlowThrough::Final::parameterValueFlow0#ffff#prev AS I ON I.<2> = I.<3>
106958 ~5%     {3} r33 = SCAN r32 OUTPUT r32.<0>, r32.<1>, r32.<2>
106958         {3} r34 = MATERIALIZE r33 AS antijoin_rhs
0      ~0%     {3} r35 = r31 AND NOT r34(r31.<0>, r31.<2>, r31.<1>)
0      ~0%     {4} r36 = SCAN r35 OUTPUT r35.<0>, r35.<2>, r35.<1>, r35.<1>
                return r36

I'm not exactly sure what's causing the change in RA. On master we only keep the rows where contentIn and contentOut are equal, where's this is not the case after the changes in this PR.

Here's the relevant predicate differences:

Master:

  exists(/* DataFlowUtil::Node */ TIRDataFlowNode mid |
    exists(/* DataFlowImplCommon::ContentOption */ TContentOption arg3 |
      rec DataFlowImplCommon::Cached::FlowThrough::Final::parameterValueFlow#ffff(p,
                                                                                  mid,
                                                                                  contentOut,
                                                                                  arg3),
      contentOut = arg3
    ),
    DataFlowImplCommon::Cached::FlowThrough::LocalFlowBigStep::localFlowBigStep#ff(mid,
                                                                                   node)
  );

This PR:

exists(/* DataFlowUtil::Node */ TIRDataFlowNode mid |
  DataFlowImplCommon::Cached::FlowThrough::LocalFlowBigStep::localFlowBigStep#ff(mid,
                                                                                 node),
  rec DataFlowImplCommon::Cached::FlowThrough::Final::parameterValueFlow#ffff(p,
                                                                              mid,
                                                                              contentIn,
                                                                              contentOut)
);

rdmarsh2 · 2020-04-10T19:35:49Z

I'm not exactly sure what's causing the change in RA. On master we only keep the rows where contentIn and contentOut are equal, where's this is not the case after the changes in this PR.

I think the constraint on master is inferred indirectly from readStep and storeStep being none(), which isn't true after the PR. The real problem is that there are many possible mid nodes that connect a pair of p and node with the same content. That might be coming from chi nodes on parameters, but averaging 100 of those seems extreme. Could it be caused by alias/conflation of parameters?

MathiasVP · 2020-04-13T12:23:50Z

I think the constraint on master is inferred indirectly from readStep and storeStep being none(), which isn't true after the PR. The real problem is that there are many possible mid nodes that connect a pair of p and node with the same content. That might be coming from chi nodes on parameters, but averaging 100 of those seems extreme. Could it be caused by alias/conflation of parameters?

The root cause was a was a stupid mistake on my side. In a previous commit I introduced an abstract class for ParameterNodes (to split explicit and implicit parameters), and then removed the abstract'ness from the class following your comment in #3123. But (unlike Jonas) I forgot to introduce a char pred to the ParameterNode class, so parameterValueFlow0 was not restricted to only parameters.

This fixed the performance problems: https://jenkins.internal.semmle.com/job/Changes/job/CPP-Differences/1041/

MathiasVP · 2020-04-15T19:59:21Z

Here's the newest CPP-differences after merging in master after Robert's outparam PR (along with necessary flow from #3220): https://jenkins.internal.semmle.com/job/Changes/job/CPP-Differences/1043/

Performance still looks good. The only major spike in time spend is on microsoft/Chakra, but that is due to increased build time. We have a couple of new results now as well:

=== Changes per project (+new -fixed) ===

g/git/git                                                                   +1 -0
g/php/php-src                                                               +2 -0

=== Changes per query (+new -fixed) ===

cpp/path-injection                                                          +2 -0

cpp/uncontrolled-allocation-size                                            +1 -0

~~I think the one in git/git is a FP from field conflation, but the other two looks like true positives. I'll take a closer look at these tomorrow.~~
On further inspection, they all look like true positives.

jbj · 2020-04-16T13:34:29Z

Here's the newest CPP-differences after merging in master after Robert's outparam PR (along with necessary flow from #3220): https://jenkins.internal.semmle.com/job/Changes/job/CPP-Differences/1043/

That looks great!

Performance still looks good. The only major spike in time spend is on microsoft/Chakra, but that is due to increased build time.

It looks to me like there's a performance regression beyond the noise level. Looking at the analysis time diffs, the first DefaultTaintTracking-based queries in the suite are clearly taking longer than they used to. The biggest relative slowdown is on git/git, but that could be noise.

I do expect a slowdown since we're doing more work than before, and a few percent seems reasonable. But do you see any individual predicates that have slowed down and are now slower than they ought to be?

jbj

The code LGTM apart from these comments.

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp

… on review comments

jbj

The new code changes LGTM. What remains is accepting the four test changes in this repo (assuming they're good) and creating a PR in the internal repo to accept the test change there. To avoid conflicts, I suggest basing the internal PR on the mergeback PR I'm going to open within an hour or so.

MathiasVP · 2020-04-17T09:58:28Z

I do expect a slowdown since we're doing more work than before, and a few percent seems reasonable. But do you see any individual predicates that have slowed down and are now slower than they ought to be?

The query that has the biggest slowdown cpp/cpp/path-injection has has the biggest slowdown in the query phase of the evaluation. Looking at the timing stats for that particular stage locally for git/git and wireshark/wireshark (and a brief look at the timing stats for other stages) there's no predicate that immediately caught my eye as unreasonably slow.

MathiasVP · 2020-04-17T10:21:57Z

Here's the relevant internal PR: https://git.semmle.com/Semmle/code/pull/36713

MathiasVP added 4 commits March 24, 2020 22:28

C++: Demonstrate amount of field flow already present

22381f3

C++: Split parameter node class into an explicit and implicit version

a5f08e1

C++: Add field flow and accept tests

077c282

C++: Autoformat

f92dd3c

MathiasVP added C++ WIP This is a work-in-progress, do not merge yet! labels Mar 24, 2020

MathiasVP added 3 commits March 26, 2020 11:39

C++: Removed toString from PostUpdateNodes. They were more confusing …

c6c6138

…than helpful

Merge branch 'master' into ir-flow-fields

a43abaa

jbj reviewed Mar 26, 2020

View reviewed changes

MathiasVP mentioned this pull request Mar 27, 2020

Data flow: No magic in returnFlowCallableCand #3142

Merged

MathiasVP added 2 commits March 27, 2020 13:32

Merge branch 'master' into ir-flow-fields

580310f

C++: Only allow flow through non-conflated chi instructions

5ba5791

MathiasVP commented Mar 28, 2020

View reviewed changes

Include join order fix from github#3142

7fce4ce

MathiasVP added 2 commits April 2, 2020 10:45

Merge branch 'master' into ir-flow-fields

020c273

C++: Accept test

af9e05b

MathiasVP marked this pull request as ready for review April 2, 2020 09:09

MathiasVP requested a review from a team as a code owner April 2, 2020 09:09

MathiasVP added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Apr 2, 2020

C++: Add QLDoc to public classes and predicates

dda3aaa

Merge branch 'master' into ir-flow-fields

52b179a

MathiasVP added 2 commits April 8, 2020 09:16

Merge branch 'master' into ir-flow-fields

d65c52d

Merge branch 'master' into ir-flow-fields

7f5330d

MathiasVP added 2 commits April 13, 2020 14:18

C++: Add charpred to ParameterNode

945ecff

Merge branch 'master' into ir-flow-fields

daac5c5

MathiasVP added 3 commits April 13, 2020 16:19

C++: Accept test output which I previously forgot to accept

cde34c9

Merge branch 'master' into ir-flow-fields

209e084

C++: Add flow from github#3220

f02feac

jbj reviewed Apr 16, 2020

View reviewed changes

C++: Make PartialDefinitionNode private and add/update comments based…

62e2ffe

… on review comments

jbj previously approved these changes Apr 17, 2020

View reviewed changes

C++: Accept test output

8c03423

MathiasVP dismissed jbj’s stale review via 8c03423 April 17, 2020 10:03

MathiasVP removed the WIP This is a work-in-progress, do not merge yet! label Apr 17, 2020

Merge branch 'master' into ir-flow-fields

ba0429c

jbj approved these changes Apr 17, 2020

View reviewed changes

jbj merged commit 8ca6c57 into github:master Apr 17, 2020

This was referenced Apr 17, 2020

C++: IR data flow from WriteSideEffectInstructions to partial ChiInstruction operands #3220

Closed

C++: Add PostUpdateNode for updates to structs with no chi instructions #3295

Merged

MathiasVP mentioned this pull request May 20, 2020

C++: Remove field conflation caused by IR field flow #3532

Merged

C++: IR field flow #3118

C++: IR field flow #3118

Uh oh!

Conversation

MathiasVP commented Mar 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbj Mar 26, 2020

Choose a reason for hiding this comment

Uh oh!

MathiasVP Mar 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbj Mar 26, 2020

Choose a reason for hiding this comment

Uh oh!

MathiasVP commented Mar 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbj commented Mar 26, 2020

Uh oh!

MathiasVP commented Mar 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MathiasVP Mar 28, 2020

Choose a reason for hiding this comment

Uh oh!

jbj Mar 30, 2020

Choose a reason for hiding this comment

Uh oh!

MathiasVP Mar 30, 2020

Choose a reason for hiding this comment

Uh oh!

MathiasVP Mar 31, 2020

Choose a reason for hiding this comment

Uh oh!

MathiasVP commented Mar 28, 2020

Uh oh!

jbj commented Mar 30, 2020

Uh oh!

MathiasVP commented Mar 30, 2020

Uh oh!

jbj commented Mar 30, 2020

Uh oh!

MathiasVP commented Mar 30, 2020

Uh oh!

MathiasVP commented Mar 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MathiasVP commented Apr 8, 2020

Uh oh!

jbj commented Apr 8, 2020

Uh oh!

MathiasVP commented Apr 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MathiasVP commented Apr 10, 2020

Uh oh!

rdmarsh2 commented Apr 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MathiasVP commented Apr 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MathiasVP commented Apr 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbj commented Apr 16, 2020

Uh oh!

jbj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jbj left a comment

Choose a reason for hiding this comment

Uh oh!

MathiasVP commented Mar 24, 2020 •

edited

Loading

MathiasVP Mar 26, 2020 •

edited

Loading

MathiasVP commented Mar 26, 2020 •

edited

Loading

MathiasVP commented Mar 27, 2020 •

edited

Loading

MathiasVP commented Mar 31, 2020 •

edited

Loading

MathiasVP commented Apr 8, 2020 •

edited

Loading

rdmarsh2 commented Apr 10, 2020 •

edited

Loading

MathiasVP commented Apr 13, 2020 •

edited

Loading

MathiasVP commented Apr 15, 2020 •

edited

Loading