dashboard does not validate text fields

Bug #1750527 reported by John Lenton on 2018-02-20
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Snap Store
Undecided
Matias Bordese
Snapcraft
Undecided
Unassigned
review-tools
Undecided
Jamie Strandboge
snapd
High
John Lenton

Bug Description

It seems dashboard.snapcraft.io does little to no validation on the text fields (Title, Summary, and Description).

In particular, I can have \n in title and summary, and arbitrary control characters in any of them.

I'd expect the three of them to reject anything that isn't valid UTF-8; of Unicode, they should reject any control or private use character (that is: any character with class Cc or Co), and noncharacters. The exception being that description should accept \n.

--

I'm tagging as a security issue because you can currently embed escape sequences into the summary, which is displayed unquoted in 'snap find' and can thus do potentially nasty things to the user's terminal. I don't think it's a _serious_ security risk, but it's nasty.

"snap find counterintelligences" for a harmless example. Note the line drawing characters are done using VT100 drawing (i.e. DEC special characters set; it changes the character set the terminal uses).

John Lenton (chipaca) on 2018-02-20
Changed in snapd:
assignee: nobody → John Lenton (chipaca)
status: New → In Progress
importance: Undecided → High
John Lenton (chipaca) wrote :

I think I'd add U+FFFD REPLACEMENT CHARACTER to the list of 'nopes'.

John Lenton (chipaca) wrote :

The store is saving newlines as \r\n, so maybe accept \r only if followed by \n only in description.

But also, can the store stop saving newlines as \r\n :-)

John Lenton (chipaca) wrote :

The bug is private because at least VTE has in the past had issues with how it handles escapes.

The plan is: close the store bug first, then open this up and harden the other three.

Jamie Strandboge (jdstrand) wrote :

John and I discussed this a bit on IRC. We should not allow control characters in any of our fields that end up being displayed to the user in snapcraft.yaml and snap.yaml or from forms from the dashboard. Historically, control characters can be used in various attack scenarios on the user's terminal. I'm told that snapd is going to allow snap.yaml overrides for description and summary that are in the store.

We need 4 tasks: snapstore (input validation on forms and what it puts into description and summary), review-tools (check description and summary harder (at least)), snapcraft (check description and summary harder (at least)) and snapd (filter output from snap to screen).

Once the snap store is fixed, this bug can be made public since it closes any sort of attacks via control characters wrt malicious uploads. We can then harden the review-tools, snapcraft and snapd publicly.

Changed in review-tools:
status: New → Triaged
assignee: nobody → Jamie Strandboge (jdstrand)
Matias Bordese (matiasb) on 2018-02-21
Changed in snapstore:
assignee: nobody → Matias Bordese (matiasb)
Changed in snapstore:
status: New → In Progress
Matias Bordese (matiasb) on 2018-02-28
Changed in snapstore:
status: In Progress → Fix Committed
Matias Bordese (matiasb) on 2018-03-01
Changed in snapstore:
status: Fix Committed → Fix Released
John Lenton (chipaca) on 2018-03-13
information type: Private Security → Public Security
Jamie Strandboge (jdstrand) wrote :

This is now implemented in trunk.

Changed in review-tools:
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers