polymer-resin intercepts polymer property assignments
before they reach XSS-vulnerable sinks like `href="..."`
and text nodes in `<script>` elements.
This follows the instructions in WORKSPACE for adding a new bower
dependency with kaspern's tweak to use the dependency in a rule so
that it's found. //lib/js/bower_components.bzl has already been
rolled-back per those instructions.
The license is the polymer license as can be seen at
https://github.com/Polymer/polymer-resin/blob/master/LICENSE though
I'm not sure that //tools/js/bower2bazel.py recognizes it as such.
Docs for the added component are available at
https://github.com/Polymer/polymer-resin/blob/master/README.mdhttps://github.com/Polymer/polymer-resin/blob/master/getting-started.md
With this change, when I introduce an XSS vulnerability as below,
polymer-resin intercepts and stops it.
Patch that introduces a strawman vulnerability.
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
@@ -55,6 +55,10 @@
url: '/q/status:abandoned',
name: 'Abandoned',
},
+ {
+ url: location.hash.replace(/^#/, '') || 'http://example.com/#fragment_echoed_here',
+ name: 'XSS Me',
+ },
],
}];
---
Address kaspern's and paladox's comments.
---
Undo version bumps for bower dependencies.
---
Change Soy index template to parallel app/index.html.
---
update polymer-resin to version 1.1.1-beta
----
Load polymer-resin into polygerrit-ui/**/*_test.html
After this, I ran the tests with
-l chrome
-l firefox
I ran a handful of tests with -p and observed that the
console shows "initResin" is called before test cases start
executing.
These changes were done programmaticly by running the script below
(approximately) thus:
```
gerrit/ $ cd polygerrit-ui/app
app/ $ find . -name \*test.html | xargs perl hack-tests.pl
```
```
use strict;
sub removeResin($) {
my $s = $_[0];
$s =~ s@<link rel="import" href="[^"]*/polymer-resin/[^"]*"[^>]*>\n?@@;
$s =~ s@<script src="[^"]*/polymer-resin/[^"]*"></script>\n?@@;
$s =~ s@<script>\s*security\.polymer_resin.*?</script>\n?@@s;
return $s;
}
for my $f (@ARGV) {
next if $f =~ m@/bower_components/|/node_modules/@;
system('git', 'checkout', $f);
print "$f\n";
my @lines = ();
open(IN, "<$f") or die "$f: $!";
my $maxLineOfMatch = 0;
while (<IN>) {
push(@lines, $_);
# Put a marker after core loading directives.
$maxLineOfMatch = scalar(@lines)
if m@/webcomponentsjs/|/polymer[.]html\b|/browser[.]js@;
}
close(IN) or die "$f: $!";
die "$f missing loading directives" unless $maxLineOfMatch;
# Given ./a/b/c/my_test.html, $pathToRoot is "../../.."
# assuming no non-leading . or .. components in the path from find.
my $pathToRoot = $f;
$pathToRoot =~ s@^\.\/@@;
$pathToRoot =~ s@^(.*?/)?app/@@;
$pathToRoot =~ s@\/[^\/]*$@@;
$pathToRoot =~ s@[^/]+@..@g;
my $nLines = scalar(@lines);
open(OUT, ">$f") or die "$f: $!";
# Output the lines up to the last polymer-resin dependency
# loaded explicitly by this test.
my $before = join '', @lines[0..($maxLineOfMatch - 1)];
$before = removeResin($before);
print OUT "$before";
# Dump out the lines that load polymer-resin and configure it for
# polygerrit.
if (1) {
print OUT qq'<link rel="import" href="$pathToRoot/bower_components/polymer-resin/standalone/polymer-resin-debug.html"/>
<script>
security.polymer_resin.install({allowedIdentifierPrefixes: [\'\']});
</script>
';
}
# Emit any remaining lines.
my $after = join '', @lines[$maxLineOfMatch..$#lines];
$after = removeResin($after);
$after =~ s/^\n*//;
print OUT "$after";
close(OUT) or die "$f: $!";
}
```
---
update polymer-resin to version 1.2.1-beta
---
update Soy index template to new style polymer-resin initialization
----
fix lint warnings
----
Load test/common-test-setup.html into *_test.html
Instead of inserting instructions to load and initialize polymer-resin into
every test file, add a common-test-setup.html that does that and also fold
iron-test-helpers loading into it.
----
imported files do not need to load webcomponentsjs
Change-Id: I71221c36ed8a0fe7f8720c1064a2fcc9555bb8df
Double-clicking selects the word underneath it on OS X and Linux text
editors. Triple-clicking selects the whole line.
Selected line can be copied or commented on.
Feature: Issue 5375
Change-Id: I79fff57e7dbfde18ab741a3a67b61869fb52cecf
- Add concept of diff comment thread groups, which are all of the
threads at a particular line number.
- The thread group is responsible for breaking up comments into threads
based on the range of the comment.
- Thread groups are ordered by the updated time of the first comment in
the group.
- Thread groups are given a key, based on comment range, which is used
to determine what thread group a new comment should go in (or if it
needs a new one).
Feature: Issue 5292
Change-Id: If544e8bb879262de3ce5397e86124837b66ada04
Diffs in PolyGerrit apply two shades of highlight to changed lines
(light and dark) to indicate the granularity of modifications and to
distinguish intraline edits. However, the logic for choosing the
background shade for diff lines would differ from that of GWT UI diffs
subtly.
+----------------------------------+----------------------------------+
| GWT UI Shading Logic | PG Shading Logic (incorrect) |
+----------------------------------+----------------------------------+
| Diff lines get a dark background | Diff lines get a dark background |
| IFF they appear in a delta chunk | IFF they do NOT contain any |
| that is empty on the left OR | intraline differences. |
| empty on the right. | |
+----------------------------------+----------------------------------+
| Diff lines get a light background otherwise. |
+---------------------------------------------------------------------+
With this change, the shading logic in PolyGerrit is modified to match
that of the GWT UI.
Bug: Issue 4219
Bug: Issue 5117
Change-Id: Ice24292df777118c08c3e73f771720f8a186a183
At the time that syntax highlighting DOM was introduced, the offsets of
selection ranges had been broken. In change [1] Kasper fixed this for
GR-DIFF-HIGHLIGHT with selection normalization functions. However,
selections for copying code as implemented in GR-DIFF-SELECTION were
still un-normalized.
With this change, the normalization functionality introduced in [1] is
moved to a JS library so that it can be used by both components. Tests
are updated.
[1] I26c61ca706575ea5df6e3b7b18a27225834396e8
Change-Id: I35ab0f71a46b3fc1d7356a314a0cae856f2ef28e
This patch fixes the issue with the split text error, and prevents
a few other edge cases with text selection from occurring. Previously,
the diff-highlight made some assumptions about DOM structure that were
broken by syntax highlighting.
Bug: Issue 4389
Change-Id: I26c61ca706575ea5df6e3b7b18a27225834396e8
Introduces the gr-syntax-layer element. This element works as an
annotation layer that is configured with the diff and asynchronously
computes/applies syntax for the diff.
Introduces a custom build of Highlight.js which gr-syntax-layer makes
use of. Building the script is documented in
scripts/vendor/highlight/building.md.
The layer is connected to the annotation pipeline in gr-diff-builder as
the lowest layer and syntax processing is triggered only after a diff
has been completely rendered.
A number of styles are added to the gr-diff element for syntax markers.
Tests added for gr-syntax-layer.
Bug: Issue 3916
Change-Id: Ic33e40f4fe39dfce1a62de133cfaf32be5e3f25a
Apply diff annotations (intraline differences and comment ranges) by
executing the annotation layers in order to each line. The diff builder
maintains an ordered array of annotation layers which are communicated
to GrDiffBuilder subclass instances. The builder also listens to each
layer for notifications that annotations have changed for some line
range and re-renders (i.e. re-applies the pipeline on DIV.contentText
elements) accordingly.
Change-Id: Iea0599d4869cafaadc0974158153a91d927913e8
- Allow range comments to start at a tab.
- Preserve tabs within selected comment range.
Bug: Issue 4253
Change-Id: Ia1ebdcc2e22a1fdad9619cf9ded5db3c80ebe648
Moves the tests for gr-annotation functions into their own test file and
fixes a subtle bug regarding `splitTextNode`'s Unicode branch.
In the DOM implementation of `node.splitText`, `node` is kept in the DOM
and its `textContent` is modified, whereas the Unicode path of
`splitTextNode` would replace it with an entirely new Text node. This
led to the function behaving differently when the Node contained or
did not contain astral code-points.
With this change, `splitTextNode` more-closely behaves like `splitText`
and this behavior is captured in a new unit-test.
Change-Id: I70460694040ba9a3c49937aaafc9db261ca3be3d
Adds the `annotateElement` function to the gr-annotation library, which
applies an annotation to an element's text at the specified range as
deeply as possible.
Change-Id: I5e38a9718fc00df860e8b12a16e551ccc1b57722
Formerly, diff content elements mixed text with comment threads. For
example, a diff content node with an intraline highlight, a ranged
comment, and a gr-diff-comment-thread may have been organized as below:
TD.content
╠ #text
╠ HL (intraline difference)
║ ╚ #text
╠ #text
╠ HL.range (ranged comment highlight)
║ ╚ #text
╠ #text
╚ GR-DIFF-COMMENT-THREAD
╠ GR-DIFF-COMMENT
╚ ...
Note that the comment thread was inserted at the same level as the text
of the diff line.
With this change, the text is separated from the comment thread by
introducing a DIV to contain the text with class `contentText` as
sibling to comment threads.
TD.content
╠ DIV.contentText
║ ╠ #text
║ ╠ HL
║ ║ ╚ #text
║ ╠ #text
║ ╠ HL.range
║ ║ ╚ #text
║ ╚ #text
╚ GR-DIFF-COMMENT-THREAD
╠ GR-DIFF-COMMENT
╚ ...
Modifies the `getContentByLine` method of gr-diff-builder to return the
`DIV.contentText` element rather than the `TD.content` element which is
its parent. In most uses of this function, the text is what is needed
rather than the TD or comment thread, but in other cases, they can be
easily DOM traversed.
Change-Id: I0eded34afd3d22963252efc7eabfee290ae21a9c
It was possible to cause a JS error when creating a ranged comment that
started at the very end of the first line (selecting no content on that
line). The relevant null-guard needed an additional set of parens to
avoid evaluating the second OR operand with a bad argument in this case.
Addresses the null-guard boolean expressions in `_normalizeStart` and
`_normalizeEnd` and reduces the number of calls to `_getLength` from
thrice to once per iteration. Adds a relevant unit test.
Change-Id: I98848f9f6089fd3240bda175765770c9f9c5ba30
Changes:
- wrapping in HL now adds cssClass instead of discarding previous ones.
- getLength accounts for tab tags correctly.
- generic splitNode, potentially should be moved into util.
- more tests.
Feature: Issue 3915
Change-Id: Id8a646a5de4fd702aa112678c039df9ff8dd8c0b
Creates action box, that creates range comment on mouse down and hotkey
over selected text in diff. Makes best effort in guessing correct start
and end points for the selection.
Known issues listed as TODO items in test and code.
Feature: Issue 3915
Change-Id: I0a3e41d062e559c8cdb4b847829429f65622eb72
Listen and update diff on comment events:
- call appropriate methods to apply comment ranges on comment creation
- re-render diff on and thread comment discard
- apply highlight on comment mouse over
- remove highlight on comment mouse out
- tests for all above
Feature: Issue 3910
Change-Id: I501ddcd063407777355b9c887118fcae53dcb5f1
Utility methods for applying comment range highlights to diff, with
tests including some of the corner cases.
Feature: Issue 3910
Change-Id: Id7de2dd4ff027ce96479a2d596e9414a0cadd6bf
