feat($sce): handle URLs through the $sce service #16378

petebacondarwin · 2017-12-19T15:05:32Z

Replaces #14250

Thanks to @rjamet for the original work on this feature.

This is a large patch to handle URLs with the $sce service, similarly to HTML context.

However, to keep rough compatibility with existing apps, we need to allow URL-context
concatenation, since previously $sce contexts prevented sanitization. There's now a
set of special contexts (defined in $interpolate) that allow concatenation, in a roughly
intuitive way:

Trusted types alone are trusted, e.g. "{{safeType}}" will not be sanitized.
Any concatenation of values results in a non-trusted type that will be handled by
getTrusted once the concatenation is done, e.g. "{{ 'javascript:foo' }}" and
"javascript:{{safeType}}" will be sanitized.

This commit also introduces a new SCE context called SRC, which represents a URL
being used as a source for an image, video, audio, etc. The hierarchy is setup
so that the URL context is also a SRC context, in the same way that the RESOURCE_URL
context is also a URL (and now also a SRC) context.

Where we previously sanitized URL attributes inside the compiler, we now only apply
the $sce.URL or $sce.SRC context requirement.

When calling getTrustedSrc() a value that is not already a trusted SRC will be
sanitized using the imgSrcSanitizationWhitelist
When calling getTrustedUrl() a value that is not already a trusted URL will be
sanitized using the aHrefSanitizationWhitelist

This results in behaviour that closely matches the previous sanitization behaviour.

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

feature

What is the current behavior? (You can also link to an open issue here)

URLs are sanitized directly in the compiler or interpolator.

What is the new behavior (if this is a feature change)?

Now that sanitization is done via the SCE service.

Does this PR introduce a breaking change?
Yes

If you use attrs.$set for URL attributes there will be no automated sanitization
of the URL value. This is now in line with other contexts. If you are programmatically
writing URL values to attributes from untrusted input then you must sanitize
it yourself (possibly by calling the private $$sanitizeUri service).

Please check if the PR fulfills these requirements

The commit message follows our guidelines
Fix/Feature: Docs have been added/updated
Fix/Feature: Tests have been added; existing tests pass

Other information:

jbedard · 2017-12-19T18:31:49Z

src/ng/interpolate.js

        }
      }

+      if (concat.length === 1 && expressionPositions.length === 1) {


Can we remove the initial singleExpression assignment and change this to singleExpression = (concat.length === 1 && expressionPositions.length === 1)?

jbedard · 2017-12-19T18:42:15Z

src/ng/interpolate.js

+              // doesn't allow that.
+              $interpolateMinErr.throwNoconcat(text);
+            } else {
+              return concat.join('');


Drop this (and the next else) and just fallthru? Could then combine the condition above to trustedContext && concat.length > 1...

jbedard · 2017-12-19T18:43:59Z

src/ng/interpolate.js

-          return allOrNothing && !isDefined(value) ? value : stringify(value);
+          if (contextAllowsConcatenation && singleExpression) {
+            // No stringification in this case, to keep the trusted value until unwrapping.
+            return value;


This basically makes parseStringifyInterceptor a noop, can we avoid parseStringifyInterceptor (and any interceptor) completely in that case? Maybe only if that case is common enough that it's worth avoiding interceptors...

Difficulty is that we assign the interpolator to the parseFn before we have calculated how many items are in contat and expressionPositions so we don't know whether singleExpression is true until we have finished.

But I managed it...

If it's difficult or ugly then we can leave it out of this PR and try later since it's not really directly related to this PR

rjamet · 2017-12-20T15:20:27Z

src/ng/sanitizeUri.js

   * regular expression. If a match is found, the original url is written into the dom. Otherwise,
-   * the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM.
+   * the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM,
+   * making it inactive.


Could you maybe document that those are used in the $sce.URL sanitization now, so it's reflected in the API docs ? (and same below)

rjamet · 2017-12-20T15:22:31Z

src/ng/sce.js


-  // An URL used in a context where it does not refer to a resource that loads code. Currently
-  // unused in AngularJS.
+  // An URL used in a context where it refers to the source of media such as an image, audio, video, etc.


If you push it a little, iframes and scripts are media. I'd say "to the source of media which are not expected to run scripts such as ..." instead.

rjamet · 2017-12-20T15:23:25Z

src/ng/sce.js

+     * the context and sanitizer availablility.
+     *
+     * The contexts that can be sanitized are $sce.MEDIA_URL, $sce.URL and $sce.HTML. The first two are available
+     * by default, and the second one relies on the $sanitize service (which may be loaded through


s/second/third/

rjamet · 2017-12-20T15:24:13Z

src/ng/sce.js

-     * is available or possible.)
+     * This function will throw if the safe type isn't appropriate for this context, or if the
+     * value given cannot be accepted in the context (which might be caused by sanitization not
+     * being available, or the value being unsafe).


"the value not being recognized as safe" instead?

rjamet · 2017-12-20T15:26:43Z

src/ng/sce.js

+      if (type === SCE_CONTEXTS.MEDIA_URL) {
+        return $$sanitizeUri(maybeTrusted, true);
+      } else if (type === SCE_CONTEXTS.URL) {
+        return $$sanitizeUri(maybeTrusted);


explicit false as a second argument there ?

oh, OK then :-) I was saving bytes.

rjamet · 2017-12-20T15:27:36Z

src/ng/sanitizeUri.js

-   * the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM.
+   * the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM,
+   * making it inactive.
   *


Below in this file, I would replace the isImage argument with isMediaUrl to be clearer. That shouldn't break anything.

rjamet · 2017-12-20T15:33:18Z

src/ng/sanitizeUri.js

 */
 function $$SanitizeUriProvider() {
+
  var aHrefSanitizationWhitelist = /^\s*(https?|s?ftp|mailto|tel|file):/,


There's a tiny quirk hiding there, which is probably without consequences: mailto and tel are in the URL whitelist, but not in the MEDIA_URL. This makes sense, but "safe under URL" is supposed to be a subset of "safe under MEDIA_URL" in my interpretation of the subclassing in the $sce. I don't think anyone cares either way.

Yeah, I wondered about that. One is not really a "super-set" of the other but I needed both to be covered by RESOURCE_URL, so I had to force a hierarchy on them. Given that users can manipulate these whitelists in anyway they like, I don't feel too bad about the defaults not quite aligning.

rjamet · 2017-12-20T15:35:42Z

src/ngSanitize/sanitize.js

      var buf = [];
      htmlParser(html, htmlSanitizeWriter(buf, function(uri, isImage) {
-        return !/^unsafe:/.test($$sanitizeUri(uri, isImage));
+        return !/^unsafe:/.test($$sanitizeUri(uri), isImage);


This looks wrong to me?

but no test is failing 😱

This is because I broke the tests in https://github.com/angular/angular.js/pull/16378/files#diff-85da78d5073ece90913c9e733cfcdef4 !!!

rjamet · 2017-12-20T16:00:30Z

test/ng/compileSpec.js

-          $rootScope.testUrl = $sce.trustAsUrl('http://example.com/image2.mp4');
+        it('should accept trusted values', inject(function($rootScope, $compile, $sce) {
+          // As a MEDIA_URL URL
+          element = $compile('<' + tag + ' src="{{testUrl}}"></' + tag + '>')($rootScope);


I think the CI errors you get are due to some browser protections on assigning javascript: to the src attributes of image elements... Those are annoying, I remember hitting these on the IE family before (and that was why I didn't bother with img there).

Right, I wondered about that. I'll remove the javascript:s

rjamet · 2017-12-20T16:12:10Z

LGTM overall, nice work!

jbedard · 2017-12-20T19:12:53Z

src/ng/interpolate.js

      }

+      singleExpression = concat.length === 1 && expressionPositions.length === 1;
+      parseFns = contextAllowsConcatenation && singleExpression ?


Would this be any shorter/nicer doing...

parseFns = expressions.map(function(exp) { return $parse(exp, contextAllowsConcatenation && singleExpression ? undefined : parseStringifyInterceptor); });

I thought about that. It does mean evaluating that contextAllowsConcatenation && singleExpression for each item in concat. Also I was being cautious in case the $parse was doing parameter counting rather than just a check for undefined. If you feel strongly about it we could change it.

I think it reads a little nicer but don't feel strongly about it.

Could do var interceptor = ... outside the loop. That might also be a little more readable which I think is most important here (I don't think performance is a major concern since this isn't run per digest or anything like that).

Narretz

As far as I can say, this looks reasonable, although I find it a bit weird that the strategy for sanitizing content that is programmtically written is to use the private $$sanitizeUri service. This feels wrong. Maybe we should make the service public?

The commit message would also be clearer if this section:

Where we previously sanitized URL attributes when setting attribute value inside the
$compile service, we now only apply the $sce.URL or $sce.SRC context requirement,
and leave the $interpolate service to deal with sanitization.

would come immediately after

This is a large patch to handle URLs with the $sce service, similarly to HTML context.

Btw,

we now only apply the $sce.URL or $sce.SRC context requirement

I don't think $sce.SRC is anywhere in this patch, is this line right?

Narretz · 2017-12-21T18:06:32Z

src/ng/interpolate.js

-          value = getValue(value);
+          // In concatenable contexts, getTrusted comes at the end, to avoid sanitizing individual
+          // parts of a full URL. We don't care about losing the trustedness here.
+          // If non-concatenable contexts where there is only one expression then this interceptor


Narretz · 2017-12-21T18:17:33Z

src/ng/sce.js

+ * <div class="alert alert-warning">
+ * Be aware that `a[href]` and `img[src]` used to automatically sanitize their URLs and not pass them
+ * through {@link ng.$sce#getTrusted $sce.getTrusted}. **As of 1.7.0, this is no longer the case.**
+ * Now `getTrusted` will sanitize values for the `$sce.MEDIA_URL` and `$sce.URL` contexts.


It's not clear which context applies to what. How about "Now getTrusted will sanitize a[href] for the $sce.URL context, and img[src] for the $sce.MEDIA?URL context.

This is not strictly related to DOM nodes. What this is saying is that, previously the a[href] and img[src] did not use $sce at all but did their own sanitization and that $sce did not do anything with the $sce.URL (and now also $sce.MEDIA_URL) contexts. Now the a[href] and img[src] do nothing but require the URL or MEDIA_URL context. It is $sce that now does the sanitizing as appropriate.

Ok, but the paragraph makes it sound like (at least to me) that the second part still refers to a[href] and img[src], and as such it's confusing that the last sentence isn't clear about which context gets applied to them. So I think adding the explicit context matching is a good idea.

doc comments updated

Narretz · 2017-12-21T18:25:30Z

test/ng/sceSpecs.js

+      expect($sce.getTrustedUrl($sce.trustAsResourceUrl('javascript:foo'))).toEqual('javascript:foo');
+    }));
+
+    it('should use the $$sanitizeUri', function() {


Isn't this technically an implementation detail?

Well I see it as an interface (albeit internal) to the service. Testing it this way means that we are less tied to the implementation of the $$sanitizeUri service.

gkalpak · 2018-01-26T11:29:33Z

src/ng/sce.js

+  MEDIA_URL: 'mediaUrl',
+
+  // An URL used in a context where it does not refer to a resource that loads code.
+  // A value that is trusted as a URL is also trusted as a MEDIA_URL.


Maybe changing it to ...can also be trusted... would be more accurate.
(It conveys the hierarchy relation without saying it is trusted, because it may not be depending on the whitelists.)

gkalpak · 2018-01-26T11:37:28Z

src/ng/sce.js

- * | `$sce.JS`           | For JavaScript that is safe to execute in your application's context.  Currently, no bindings require this context.  Feel free to use it in your own directives. |
+ * | `$sce.HTML`         | For HTML that's safe to source into the application.  The {@link ng.directive:ngBindHtml ngBindHtml} directive uses this context for bindings. If an unsafe value is encountered and the {@link ngSanitize $sanitize} module is present this will sanitize the value instead of throwing an error. |
+ * | `$sce.CSS`          | For CSS that's safe to source into the application.  Currently unused.  Feel free to use it in your own directives. |
+ * | `$sce.MEDIA_URL`    | For URLs that are safe to display as media. Is automatically converted from string by sanitizing when needed. |


AFAICT, "display" does not cover all usecases (e.g. audio) and might be confusing.

I'm going with render it is vague enough to cover audio IMO.

gkalpak · 2018-01-26T11:38:58Z

src/ng/sce.js

+ * | `$sce.CSS`          | For CSS that's safe to source into the application.  Currently unused.  Feel free to use it in your own directives. |
+ * | `$sce.MEDIA_URL`    | For URLs that are safe to display as media. Is automatically converted from string by sanitizing when needed. |
+ * | `$sce.URL`          | For URLs that are safe to follow as links. Is automatically converted from string by sanitizing when needed. |
+ * | `$sce.RESOURCE_URL` | For URLs that are not only safe to follow as links, but whose contents are also safe to include in your application.  Examples include `ng-include`, `src` / `ngSrc` bindings for tags other than `IMG` (e.g. `IFRAME`, `OBJECT`, etc.)  <br><br>Note that `$sce.RESOURCE_URL` makes a stronger statement about the URL than `$sce.URL` does and therefore contexts requiring values trusted for `$sce.RESOURCE_URL` can be used anywhere that values trusted for `$sce.URL` are required. |


Maybe you could mention MEDIA_URL here or in $sce.URL.

gkalpak · 2018-01-26T12:33:07Z

test/ng/interpolateSpec.js

+
+
+    it('should interpolate and sanitize a multi-part expression when isTrustedContext is URL', inject(function($sce, $interpolate) {
+      /* jshint scripturl:true */


jshint???

:-) picked up from @rjamet's original PR that was written before we moved to eslint. Removed

gkalpak · 2018-01-26T12:35:52Z

test/ng/directive/booleanAttrsSpec.js

-      element = $compile('<a ng-href="http://www.google.com/{{\'a%link\'}}">')($rootScope);
-
      expect(function() {
+        element = $compile('<a ng-href="http://www.google.com/{{\'a%link\'}}">')($rootScope);


Is this a style-only change, or has the behavior changed?

The behaviour has changed. The throw is now in the call to $compile.

gkalpak · 2018-01-26T12:46:46Z

src/ng/sanitizeUri.js

+   * the $sce.MEDIA_URL security context. When interpolation occurs a call is made to `$sce.trustAsUrl(url)`
+   * which in turn may call `$$sanitizeUri(url, isMedia)` to sanitize the potentially malicious URL.
+   *
+   * If the URL to matches the `aHrefSanitizationWhitelist` regular expression, it is returned unchanged.


URL to matches --> URL matches
aHrefSanitizationWhitelist --> imgSrcSanitizationWhitelist

gkalpak · 2018-01-26T12:51:35Z

src/ng/sanitizeUri.js

-      var normalizedVal;
-      normalizedVal = urlResolve(uri && uri.trim()).href;
+    return function sanitizeUri(uri, isMediaUrl) {
+      if (!uri) return uri;


AFAICT, this changes the current behavior. Previously, even if the URL was the empty string, it might return a non-empty URL (i.e. the current URL). Do I miss something? Is this change intended?

Hmm, you're right that urlResolve doesn't (mostly) return the empty string when passed an empty string... I need to look again at why I did this. There was definitely a reason.

I can't remember (or see) why I did this. So I am going to revert that change.

gkalpak · 2018-01-26T12:53:51Z

src/ng/directive/attrs.js

          // then calling element.setAttribute('src', 'foo') doesn't do anything, so we need
          // to set the property as well to achieve the desired effect.
-          // We use attr[attrName] value since $set can sanitize the url.
+          // We use attr[attrName] value since $set might have sanitized the url.


Based on the commit message, I thought $set() doesn't sanitize it any more 😕

We still sanitize img[srcset] when called through $set.

gkalpak · 2018-01-26T13:02:28Z

src/ng/compile.js

-            (nodeName === 'img' && key === 'src') ||
-            (nodeName === 'image' && key === 'xlinkHref')) {
-          // sanitize a[href] and img[src] values
-          this[key] = value = $$sanitizeUri(value, nodeName === 'img' || nodeName === 'image');


So, doesn't this leave people using ng-href/src ~~less~~more vulnerable (unless they use interpolation)?
(Not common to use ng-href/src without interpollation, but possible.)

I am not sure what you mean here. Do you mean "more" vulnerable?
It is true that if you were programmatically setting the attribute via $set then it is no longer checked, but this is quite a common paradigm, where one has to be responsible for coding against the lower level API.

Yeah, I meant "more" (everybody knows "less" is "more" 😛)
The thing is that some of the built-in directives (e.g. ngHref/ngSrc) use $set under the hood.

I am pretty certain that ngHref and ngSrc are not affected. They only call $set with a value that is passed from $observe, which has already interpolated the value, and so has passed the value through SCE.

gkalpak · 2018-01-26T13:06:51Z

docs/content/error/$compile/srcset.ngdoc

+passing a "trusted" value provided by calls to `$sce.trustAsMediaUrl(value)`.
+
+If you want to programmatically set explicitly trusted unsafe URLs, you should use `$sce.trustAsHtml`
+on the whole `img` tag and inject it into the DOM using the `ng-bind-html` directive.


gkalpak · 2018-01-29T16:57:56Z

I am a little concerned about #16378 (comment).
Other than that LGTM (as long as Travis is happy 😉)!

petebacondarwin · 2018-01-30T14:09:13Z

src/ng/interpolate.js

-          expressionPositions = [],
-          singleExpression,
-          contextAllowsConcatenation = trustedContext === $sce.URL || trustedContext === $sce.MEDIA_URL;
+      endIndex,


Yes, yes, indentation 😱

petebacondarwin · 2018-01-30T14:09:50Z

test/ng/directive/booleanAttrsSpec.js

 });
-
-
-describe('ngSrc', function() {


All these tests here and below were in the wrong file. I have moved them to their appropriate places.

petebacondarwin · 2018-01-30T14:10:40Z

test/ng/directive/ngHrefSpec.js

@@ -0,0 +1,104 @@
+'use strict';
+
+fdescribe('ngHref', function() {


Agh! fdescribe!!

petebacondarwin · 2018-01-30T14:10:49Z

test/ng/directive/ngHrefSpec.js

+    element = $compile('<a ng-href="javascript:alert(1);">')($rootScope);
+    $rootScope.$digest();
+    expect(element.attr('href')).toBe('unsafe:javascript:alert(1);');
+  }));


This is the key new test for this commit

petebacondarwin · 2018-01-30T14:11:06Z

test/ng/directive/ngSrcSpec.js

+      element = $compile('<img ng-src="javascript:alert(1);">')($rootScope);
+      $rootScope.$digest();
+      expect(element.attr('src')).toBe('unsafe:javascript:alert(1);');
+    }));


And this test too.

gkalpak

LGTM (as soon as Travis is green)

@rjamet

Thanks to @rjamet for the original work on this feature. This is a large patch to handle URLs with the $sce service, similarly to HTML context. Where we previously sanitized URL attributes when setting attribute value inside the `$compile` service, we now only apply an `$sce` context requirement and leave the `$interpolate` service to deal with sanitization. This commit introduces a new `$sce` context called `MEDIA_URL`, which represents a URL used as a source for a media element that is not expected to execute code, such as image, video, audio, etc. The context hierarchy is setup so that a value trusted as `URL` is also trusted in the `MEDIA_URL` context, in the same way that the a value trusted as `RESOURCE_URL` is also trusted in the `URL` context (and transitively also the `MEDIA_URL` context). The `$sce` service will now automatically attempt to sanitize non-trusted values that require the `URL` or `MEDIA_URL` context: * When calling `getTrustedMediaUrl()` a value that is not already a trusted `MEDIA_URL` will be sanitized using the `imgSrcSanitizationWhitelist`. * When calling `getTrustedUrl()` a value that is not already a trusted `URL` will be sanitized using the `aHrefSanitizationWhitelist`. This results in behaviour that closely matches the previous sanitization behaviour. To keep rough compatibility with existing apps, we need to allow concatenation of values that may contain trusted contexts. The following approach is taken for situations that require a `URL` or `MEDIA_URL` secure context: * A single trusted value is trusted, e.g. `"{{trustedUrl}}"` and will not be sanitized. * A single non-trusted value, e.g. `"{{ 'javascript:foo' }}"`, will be handled by `getTrustedMediaUrl` or `getTrustedUrl)` and sanitized. * Any concatenation of values (which may or may not be trusted) results in a non-trusted type that will be handled by `getTrustedMediaUrl` or `getTrustedUrl` once the concatenation is complete. E.g. `"javascript:{{safeType}}"` is a concatenation of a non-trusted and a trusted value, which will be sanitized as a whole after unwrapping the `safeType` value. * An interpolation containing no expressions will still be handled by `getTrustedMediaUrl` or `getTrustedUrl`, whereas before this would have been short-circuited in the `$interpolate` service. E.g. `"some/hard/coded/url"`. This ensures that `ngHref` and similar directives still securely, even if the URL is hard-coded into a template or index.html (perhaps by server-side rendering). BREAKING CHANGES: If you use `attrs.$set` for URL attributes (a[href] and img[src]) there will no longer be any automated sanitization of the value. This is in line with other programmatic operations, such as writing to the innerHTML of an element. If you are programmatically writing URL values to attributes from untrusted input then you must sanitize it yourself. You could write your own sanitizer or copy the private `$$sanitizeUri` service. Note that values that have been passed through the `$interpolate` service within the `URL` or `MEDIA_URL` will have already been sanitized, so you would not need to sanitize these values again.

googlebot added the cla: yes label Dec 19, 2017

petebacondarwin requested review from IgorMinar, gkalpak and rjamet December 19, 2017 15:05

petebacondarwin added needs: review severity: security type: feature labels Dec 19, 2017

petebacondarwin added this to the 1.7.0 milestone Dec 19, 2017

petebacondarwin mentioned this pull request Dec 19, 2017

feat($sce): handle URLs through the $sce service, plus allow concatenation in that $sce contexts. #14250

Closed

3 tasks

petebacondarwin force-pushed the sceUrl branch from 36c4de9 to c59787d Compare December 19, 2017 16:22

jbedard reviewed Dec 19, 2017

View reviewed changes

petebacondarwin force-pushed the sceUrl branch 2 times, most recently from a1611a4 to 84750b9 Compare December 20, 2017 13:44

rjamet reviewed Dec 20, 2017

View reviewed changes

jbedard reviewed Dec 20, 2017

View reviewed changes

Narretz suggested changes Dec 21, 2017

View reviewed changes

petebacondarwin force-pushed the sceUrl branch 3 times, most recently from 974f10e to 54c8c0f Compare December 22, 2017 12:51

petebacondarwin force-pushed the sceUrl branch 6 times, most recently from 6389ae3 to 90952ba Compare December 23, 2017 10:09

gkalpak reviewed Jan 26, 2018

View reviewed changes

petebacondarwin force-pushed the sceUrl branch 2 times, most recently from 04b6bed to 7202231 Compare January 29, 2018 11:09

petebacondarwin commented Jan 30, 2018

View reviewed changes

petebacondarwin force-pushed the sceUrl branch 2 times, most recently from 044a801 to 2848899 Compare January 30, 2018 14:40

gkalpak approved these changes Jan 30, 2018

View reviewed changes

petebacondarwin force-pushed the sceUrl branch from 2848899 to e541a33 Compare January 31, 2018 12:36

petebacondarwin merged commit 1e9eadc into angular:master Jan 31, 2018

petebacondarwin deleted the sceUrl branch January 31, 2018 13:13

Narretz mentioned this pull request Aug 15, 2018

fix(ngHref): allow numbers as input #16652

Merged

3 tasks



		it('should interpolate and sanitize a multi-part expression when isTrustedContext is URL', inject(function($sce, $interpolate) {
		/* jshint scripturl:true */

		@@ -0,0 +1,104 @@
		'use strict';

		fdescribe('ngHref', function() {

feat($sce): handle URLs through the $sce service #16378

feat($sce): handle URLs through the $sce service #16378

Uh oh!

Conversation

petebacondarwin commented Dec 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rjamet Dec 20, 2017 • 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!

Choose a reason for hiding this comment

Uh oh!

rjamet commented Dec 20, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Narretz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Dec 19, 2017 •

edited

Loading

rjamet Dec 20, 2017 •

edited

Loading

petebacondarwin Jan 28, 2018 •

edited

Loading