1218402 - [Messages] Better sanitize created links

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Julien Wajsberg [:julienw]]

See [1] https://github.com/mozilla-b2g/gaia/blob/b93fb040d2a0981aa3a3d9e59225e9b11cf017bd/apps/sms/views/conversation/js/link_helper.js#L164-L165

I don't feel comfortable with this code, I think we should strenghten this.

Comment 1

[Stephanie Ouillon [:arroway] (needinfo me)]

So according to the comment, the "|inputText| should be already html escaped" once we access this part of the code.
In conversation.js [1], when the transform function is called, the content is escaped before calling "LinkHelper.searchAndLinkClickableData(text)" (which then calls the code you're pointing at).

I think it would be "safer" (as in "is less likely to be forgotten by the developer") to escape the input in the searchAndLinkClickableData() function in link_helper.js (or in all the transform() functions).

[1] https://github.com/mozilla-b2g/gaia/blob/b93fb040d2a0981aa3a3d9e59225e9b11cf017bd/apps/sms/views/conversation/js/conversation.js#L1492

Comment 2

[Stephanie Ouillon [:arroway] (needinfo me)]

As for strenghtening it, the matchFilter above looks like doing the job.

Comment 3

[Julien Wajsberg [:julienw]]

Thanks Stephanie; you're right, looks like that there is no current security vulnerability. Thanks for the analysis !

([1] is the place we do the same for SMS)

[1] https://github.com/mozilla-b2g/gaia/blob/b93fb040d2a0981aa3a3d9e59225e9b11cf017bd/apps/sms/views/conversation/js/conversation.js#L1670-L1671

Not sure where it's better to escape the content. Actually I'd say that the function should be rewritten as a generator so that the new links are already escaped inside the function, but we can escape the text that is not link-ified.

Comment 4

[Julien Wajsberg [:julienw]]

As Stéphanie commented, we have no security risk currently. So removing nom.

Comment 5

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(

Comment 6

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(