Urlencoding strip markers

List overview All Threads
Download

newer

older

Wikitech.wikimedia.org read-only,...

Re: [Wikitech-l] [Engineering]...

Arlo Breault

29 Jan 2015 29 Jan '15

10:47 p.m.

Currently, while {{urlencod}}ing, content in strip markers is skipped. I believe this violates the expectation that the entire output will be properly escaped to be placed in a sensitive context. An example is in the infobox book caption on, https://en.wikipedia.org/wiki/%22F%22_Is_for_Fugitive There’s a brief discussions of the security implications of some proposed solutions in the review of, https://gerrit.wikimedia.org/r/#/c/181519/ It seems best (I guess) to just drop the content (`killMarkers()`). Any opinions or better ideas? Thanks, Arlo

Show replies by date

Brad Jorsch (Anomie)

30 Jan 30 Jan

1:38 a.m.

On Thu, Jan 29, 2015 at 2:47 PM, Arlo Breault <abreault(a)wikimedia.org> wrote:

...

There’s a brief discussions of the security implications of some proposed solutions in the review of, https://gerrit.wikimedia.org/r/#/c/181519/

To clarify, the possible solutions seem to be: 1. Unstrip the marker and then encode the content. This is a security hole (T73167) 2. Encode the marker. This results in strip markers in the output. 3. Ignore the marker. This leaves non-encoded content in the middle of what is supposed to be encoded content. 4. Remove the marker. This loses whatever is inside the marker. 5. Just output an error, to make it obvious something stupid is going on. There's no good option, so which of 2, 3, 4, and 5 is least bad? -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation

Brion Vibber

7:02 p.m.

On Thu, Jan 29, 2015 at 5:38 PM, Brad Jorsch (Anomie) <bjorsch(a)wikimedia.org

...

wrote:

> On Thu, Jan 29, 2015 at 2:47 PM, Arlo Breault <abreault(a)wikimedia.org>

...

wrote:

> > https://gerrit.wikimedia.org/r/#/c/181519/ > > > > To clarify, the possible solutions seem to be: > > 1. Unstrip the marker and then encode the content. This is a security hole > (T73167) > I'd be inclined to unstrip the marker *and squash HTML to plaintext*, then encode the plaintext... -- brion

...

2. Encode the marker. This results in strip markers in the output. 3. Ignore the marker. This leaves non-encoded content in the middle of what is supposed to be encoded content. 4. Remove the marker. This loses whatever is inside the marker. 5. Just output an error, to make it obvious something stupid is going on. There's no good option, so which of 2, 3, 4, and 5 is least bad? -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation _______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Jackmcbarn

8:11 p.m.

On Fri, Jan 30, 2015 at 2:02 PM, Brion Vibber <bvibber(a)wikimedia.org> wrote:

...

On Thu, Jan 29, 2015 at 5:38 PM, Brad Jorsch (Anomie) < bjorsch(a)wikimedia.org

wrote:

> On Thu, Jan 29, 2015 at 2:47 PM, Arlo Breault <abreault(a)wikimedia.org>

wrote:

> > https://gerrit.wikimedia.org/r/#/c/181519/ > > > > To clarify, the possible solutions seem to be: > > 1. Unstrip the marker and then encode the content. This is a security hole

(T73167)

I'd be inclined to unstrip the marker *and squash HTML to plaintext*, then encode the plaintext...

I don't see how that addresses the security issue.

Brion Vibber

9:04 p.m.

On Fri, Jan 30, 2015 at 12:11 PM, Jackmcbarn <jackmcbarn(a)gmail.com> wrote:

...

On Fri, Jan 30, 2015 at 2:02 PM, Brion Vibber <bvibber(a)wikimedia.org> wrote:

On Thu, Jan 29, 2015 at 5:38 PM, Brad Jorsch (Anomie) < bjorsch(a)wikimedia.org

wrote:

> On Thu, Jan 29, 2015 at 2:47 PM, Arlo Breault <abreault(a)wikimedia.org>

wrote:

> > https://gerrit.wikimedia.org/r/#/c/181519/ > > > > To clarify, the possible solutions seem to be: > > 1. Unstrip the marker and then encode the content. This is a security hole

(T73167)

I'd be inclined to unstrip the marker *and squash HTML to plaintext*,

then

encode the plaintext...

I don't see how that addresses the security issue.

Rollback tokens in the Special:Contributions HTML would then not be available in the squashed text that got encoded. Thus it could not be extracted and used in the timing attack. -- brion

Arlo Breault

3 Feb 3 Feb

6:01 p.m.

On Friday, January 30, 2015 at 1:04 PM, Brion Vibber wrote:

...

On Fri, Jan 30, 2015 at 12:11 PM, Jackmcbarn <jackmcbarn(a)gmail.com (mailto:jackmcbarn@gmail.com)> wrote:

On Fri, Jan 30, 2015 at 2:02 PM, Brion Vibber <bvibber(a)wikimedia.org (mailto:bvibber@wikimedia.org)> wrote:

On Thu, Jan 29, 2015 at 5:38 PM, Brad Jorsch (Anomie) < bjorsch(a)wikimedia.org (mailto:bjorsch@wikimedia.org)

wrote:

> On Thu, Jan 29, 2015 at 2:47 PM, Arlo Breault <abreault(a)wikimedia.org (mailto:abreault@wikimedia.org)>

wrote:

> > https://gerrit.wikimedia.org/r/#/c/181519/ > > > > To clarify, the possible solutions seem to be: > > 1. Unstrip the marker and then encode the content. This is a security hole

(T73167)

I'd be inclined to unstrip the marker *and squash HTML to plaintext*,

then

encode the plaintext...

I don't see how that addresses the security issue.

Rollback tokens in the Special:Contributions HTML would then not be available in the squashed text that got encoded. Thus it could not be extracted and used in the timing attack.

Is this what you mean by “squash HTML to plaintext”? urlencode( strip_tags( $parser->mStripState->unstripBoth( $s ) ) ); Is strip_tags reliable enough to not get confused and leave those tokens lying around?

...

-- brion _______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org (mailto:Wikitech-l@lists.wikimedia.org) https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Brad Jorsch (Anomie)

6:11 p.m.

On Fri, Jan 30, 2015 at 4:04 PM, Brion Vibber <bvibber(a)wikimedia.org> wrote:

...

On Fri, Jan 30, 2015 at 12:11 PM, Jackmcbarn <jackmcbarn(a)gmail.com> wrote:

On Fri, Jan 30, 2015 at 2:02 PM, Brion Vibber <bvibber(a)wikimedia.org> wrote:

I'd be inclined to unstrip the marker *and squash HTML to plaintext*,

then

encode the plaintext...

I don't see how that addresses the security issue.

Rollback tokens in the Special:Contributions HTML would then not be available in the squashed text that got encoded. Thus it could not be extracted and used in the timing attack.

Brion Vibber

6:24 p.m.

Special page inclusions shouldn't be able to do anything privileged; they're meant for public data. If that's not being enforced right now I'd recommend reworking or killing the special page inclusion system... -- brion On Feb 3, 2015 10:11 AM, "Brad Jorsch (Anomie)" <bjorsch(a)wikimedia.org> wrote:

...

On Fri, Jan 30, 2015 at 4:04 PM, Brion Vibber <bvibber(a)wikimedia.org> wrote:

On Fri, Jan 30, 2015 at 12:11 PM, Jackmcbarn <jackmcbarn(a)gmail.com>

wrote:

On Fri, Jan 30, 2015 at 2:02 PM, Brion Vibber <bvibber(a)wikimedia.org> wrote:

I'd be inclined to unstrip the marker *and squash HTML to plaintext*,

then

encode the plaintext...

I don't see how that addresses the security issue.

Rollback tokens in the Special:Contributions HTML would then not be available in the squashed text that got encoded. Thus it could not be extracted and used in the timing attack.

While it would avoid *this* bug, it would still allow the attack if there is ever sensitive data on some transcludable special page that isn't embedded in HTML tag attributes. -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation _______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Arlo Breault

10 Feb 10 Feb

2:13 a.m.

On Tuesday, February 3, 2015 at 10:24 AM, Brion Vibber wrote:

...

Ok, although Brion's idea preserves more of the original content, these larger security concerns don’t look like they are going to be resolved in short order. I think the pragmatic thing to do is either drop the content and raise an error, or replace the content with a warning string as Gergo suggested. Any takers?

...

-- brion On Feb 3, 2015 10:11 AM, "Brad Jorsch (Anomie)" <bjorsch(a)wikimedia.org (mailto:bjorsch@wikimedia.org)> wrote:

On Fri, Jan 30, 2015 at 4:04 PM, Brion Vibber <bvibber(a)wikimedia.org (mailto:bvibber@wikimedia.org)> wrote:

On Fri, Jan 30, 2015 at 12:11 PM, Jackmcbarn <jackmcbarn(a)gmail.com (mailto:jackmcbarn@gmail.com)>

wrote:

On Fri, Jan 30, 2015 at 2:02 PM, Brion Vibber <bvibber(a)wikimedia.org (mailto:bvibber@wikimedia.org)> wrote: > I'd be inclined to unstrip the marker *and squash HTML to plaintext*, then > encode the plaintext... I don't see how that addresses the security issue.

Rollback tokens in the Special:Contributions HTML would then not be available in the squashed text that got encoded. Thus it could not be extracted and used in the timing attack.

While it would avoid *this* bug, it would still allow the attack if there is ever sensitive data on some transcludable special page that isn't embedded in HTML tag attributes. -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation _______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org (mailto:Wikitech-l@lists.wikimedia.org) https://lists.wikimedia.org/mailman/listinfo/wikitech-l

_______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org (mailto:Wikitech-l@lists.wikimedia.org) https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Gergo Tisza

30 Jan 30 Jan

7:44 p.m.

On Thu, Jan 29, 2015 at 5:38 PM, Brad Jorsch (Anomie) <bjorsch(a)wikimedia.org

...

wrote:

...

4. Remove the marker. This loses whatever is inside the marker. 5. Just output an error, to make it obvious something stupid is going on.

Failing loud and early is generally a good idea, but if we don't want that, maybe replace the marker with STRIP-MARKER-WAS-REMOVED-SEE-BUG-12345? That at least gives a fighting chance to editors trying to debug why the template does not work as expected.

3387

days inactive

3399

days old

wikitech-l@lists.wikimedia.org

Manage subscription

9 comments

5 participants

tags (0)

participants (5)

Arlo Breault
Brad Jorsch (Anomie)
Brion Vibber
Gergo Tisza
Jackmcbarn