HTML injection in phenogrid tooltips (and <AppNodeBadge> more generally)

[ptgolden]

As described in #887 (comment), there is an HTML injection bug in the phenogrid component. The offending code is here:

monarch-app/frontend/src/components/ThePhenogrid.vue

Lines 183 to 203 in ce0851b

	<AppNodeBadge
	v-if="cell.phenotype"
	:node="{ id: cell.col.id, name: cell.col.label }"
	:absolute="true"
	/>
	<div class="tooltip-heading">
	<AppNodeBadge
	v-if="cell.phenotype"
	:node="{ id: cell.phenotype, name: cell.phenotypeLabel }"
	:absolute="true"
	/>
	<AppNodeBadge
	v-else
	:node="{ id: cell.col.id, name: cell.col.label }"
	:absolute="true"
	/>
	<AppIcon icon="arrows-left-right" />
	<AppNodeBadge
	:node="{ id: cell.row.id, name: cell.row.label }"
	:absolute="true"
	/>

The name property in <AppNodeBadge> (which can be user generated for columns in the multicompare mode of phenogrid) is passed as raw HTML here:

monarch-app/frontend/src/components/AppNodeBadge.vue

Lines 13 to 27 in ce0851b

	<AppLink
	v-if="isLink"
	:to="`${absolute ? baseurl : ''}${node.id}`"
	:state="
	breadcrumbs
	? { breadcrumbs: [...currentBreadcrumbs, ...breadcrumbs] }
	: state || undefined
	"
	v-html="name"
	></AppLink>
	<span v-else>
	<span class="name" v-html="name"></span>
	<span v-if="info">({{ info }})</span>
	</span>
	</span>

This breaks on labels that contain strings that can be parsed as HTML tags, such as 66.45-Dsg2<tm1a(EUCOMM)Wtsi>/Dsg2<tm1a(EUCOMM)Wtsi>, or, for script injection fun, Group Label <script type="text/javascript">console.alert('hi')</script>. As a rule, it is not good to render user-generated text directly as HTML. (The Vue documentation agrees).

This issue would also show itself if there were ever an entity in the monarch KG that had a name with some <angled brackets> in it.

[ptgolden]

The injection bug was introduced in c106f34.

@kevinschaper, do you remember why you decided you needed to pass HTML rather than text? It looks like it's to support entity labels that contain HTML, like Lama2<sup>tm1Stk</sup>/Lama2<sup>tm1Stk</sup> [background:] involves: 129/Sv * 129P2/OlaHsd * BALB/c * ICR. Is it the case that labels are allowed to contain any arbitrary HTML?

[kevinschaper]

Yeah, it’s for display of html in genotype names, but it really is more that we want to allow sup tags than any html.

[kevinschaper]

I’m used to there being both html and ascii representations of genotypes, and it makes sense that they kind of require the opposite setting. Right now we’re using provided genotype names that are the html version where alleles are in sup tags (example here), and IMPC is passing the ascii equivalent where the alleles are in angle brackets.

[ptgolden]

In that case, a safer way to do that would be to only render <sup> tags rather than any arbitrary HTML. I wrote a component that does just that:

<AppNodeText text="this has <sup>superscripted text</sup> contained in it and also <some> other stuff</some>" />

renders as:

[ptgolden]

It's also worth noting that the phenogrid is rendered with SVG, and <sup> is not an SVG tag

[ptgolden]

Nice, got it working in SVG too:

[ptgolden]

There are a couple more places where v-html is used to render HTML from the KG.

These two are node names, so they can use the new <AppNodeText> component:

monarch-app/frontend/src/pages/node/SectionTitle.vue

Lines 27 to 31 in ce0851b

	<span
	:style="{ textDecoration: node.deprecated ? 'line-through' : '' }"
	v-html="node.name"
	>
	</span>

monarch-app/frontend/src/pages/node/SectionOverview.vue

Lines 74 to 78 in ce0851b

	<p
	class="truncate-2"
	tabindex="0"
	v-html="node.synonym?.join(',\n&nbsp;')"
	></p>

This one is a node's description. @kevinschaper, can you point to an example where a node description has HTML in it?

monarch-app/frontend/src/pages/node/SectionOverview.vue

Lines 30 to 35 in ce0851b

	<p
	v-tooltip="'Click to expand'"
	class="description truncate-10"
	tabindex="0"
	v-html="node.description?.trim()"
	></p>

[ptgolden]

Oh, I see that there are a couple thousand entities with <i> in their name. I'll render those tags too. Here are some examples:

    {
        "name":"LEW-<i>Chrna4<sup>em4Mcwi</sup></i>"},
      {
        "name":"SS.BN-(<i>D13Mgh13-D13Mit4</i>)/Mcwi"},
      {
        "name":"SS-<i>Arhgef11<sup>em4Mcwi</sup></i>"},
      {
        "name":"SS.MNS-(<i>D10Mit11-D10M11Mit119</i>)/Mco"},
      {
        "name":"ACI.FHH-(<i>D1Rat324-D1Rat156</i>)/Eur"},
      {
        "name":"WF.BBDR-(<i>D4Arb29-D4Rat96</i>)/Wor"},
      {
        "name":"LEW.BN-(<i>D10Got9-D10Rat2</i>)/Rj"},
      {
        "name":"WF.BBDR-(<i>D4Rat96-D4Rat44</i>)/Wor"},
      {
        "name":"F344.HTX-(<i>D11Rat4-D11Arb4</i>)/Hcj"},
      {
        "name":"WF.DA-(<i>D19Mit1-D19Mit6</i>)/Kop"},
      {
        "name":"LEW.BN-(<i>D10Mgh7-D10Rat221</i>)/Ciml"},
      {
        "name":"PCK-<i>Pkhd1<sup>pck</i></sup>/CrljCrl"},
      {
        "name":"DA.ACI-(<i>D15Rat6-D15Rat13</i>)/Kini"},
      {
        "name":"FHH-<i>Adra1a<sup>m1Mcwi</i></sup>"},
      {
        "name":"BN-Lcat<i><sup>m3Mcwi</i></sup>"},
      {
        "name":"SS-<i>Kcna5<sup>m1Mcwi</i></sup>"},
      {
        "name":"BN.OLETF-(<i>D1Rat169-D1Rat90</i>)/Got"},
      {
        "name":"SHRSP.WKY-<i>(D3Rat227-D3Rat1)</i>/Tkyo"},

[ptgolden]

There are 41 nodes that have HTML in the description. Additionally, there are two that include the text "..." that will be misrendered if passed as HTML

text that will be misrendered if rendered as HTML (2)

Catalysis of the reaction: <stereo>cis,trans</stereo>-4-hydroxymuconate semialdehyde + H2O + NAD+ = 2 H+ + maleylacetate + NADH.

Catalysis of the reaction: oxaloacetate = <stereo>enol</stereo>-oxaloacetate.

<i> tags (2)

A rare, genetic, developmental defect during embryogenesis disorder characterized by severe, early-onset, salt-wasting adrenal insufficiency and ambiguous/female external genitalia (irrespective of chromosomal sex) due to mutations in the <i>CYP11A1</i> gene. Milder cases may present delayed onset of adrenal gland dysfunction and genitalia phenotype may range from normal male to female in individuals with 46,XY karyotype. Imaging studies reveal hypoplastic/absent adrenal glands and biochemical findings include low serum cortisol, mineralocorticoids, androgens, and sodium, with elevated potassium levels.

A rare disorder due to poisoning characterized by acute onset of potentially life-threatening illness following ingestion, inhalation, or injection of ricin, a lectin present in the seeds of <i>Ricinus communis</i>, the castor oil plant. Clinical presentation depends on the route of administration, inhalation being the most toxic route, followed by oral ingestion. Presenting signs and symptoms include nausea, vomiting, diarrhea, hematemesis, and melena (upon ingestion), cough, wheezing, dyspnea, sore throat, and congestion (upon inhalation), and erythema, induration, blisters, capillary leak syndrome, and localized necrosis (upon injection). The condition can progress to seizures, shock, organ failure, pulmonary edema, and respiratory failure.

<a> tags (39)

Patch of thickened, pseudostratified epithelium that is associated with the lateral semicircular canal. It consists of regular arrays of sensory hair cells interspersed with supporting cells. The lateral crista has its own characteristic shape and polarity pattern. (See Anatomical Atlas entry for <a href='http://zfin.org/zf_info/anatomy/dict/sens/sens.html'>sensory patches of the ear</a> by T. Whitfield.)

Macula that is located in the developing anterior inner ear, and gives rise to the macula utricle. (See Anatomical Atlas entry for <a href='http://zfin.org/zf_info/anatomy/dict/sens/sens.html'>sensory patches of the ear</a> by T. Whitfield.)