TIM-JYU / TIM

TIM (The Interactive Material) is an open-source cloud-based platform for creating interactive learning documents.
https://tim.education/view/about/en-US
MIT License
13 stars 4 forks source link

Tyylien sanitointi (ainakin kommentteissa) #2169

Closed dezhidki closed 2 years ago

dezhidki commented 3 years ago

Teen tästä ensin tiketin, sillä en ole varma muutoksen seurauksista.

Tällä hetkellä htmlSanitize sallii style-elementin kirjoittamista Markdowniin. Tällä hetkellä tätä sanitointia käytetään mm. dokumenttilohkojen ja kommenttien yhteydessä.

Koska kommentit sanitoidaan samalla tavalla, tämä tarkoittaa, että kuka tahansa voi upottaa style-elementteja mille tahansa sivulle, jossa kommentoiti on sallittu (vaikkapa Ohj1 luentomoniste).

Tämä mahdollistaa erilaisia pienempiä ja suurempia kiusallisia sivun ulkonäön muutoksia.

Proof of concept

Tein yksinkertaisen mallisivun, jonka sisältö on

# Hello, world

This is a test

ja sallin kirjautuneiden käyttäjien kirjoittaa kommentteja.

Tämän jälkeen lisäsin kommentin, jonka sisällöksi asetin

Why is this header red?!?!?!
<style>
h1 {
 color: red !important;
}
</style>

Tämän jälkeen kun päivitin sivun, h1-tyyli valui sivun sisältöön:

image

Sivu livenä: https://tim.jyu.fi/view/users/dezhidki/evil/commentdemo1

Katso sivun managesta, että sivulla ei ole mitään tyyliasetuksia kuin se, joka tulee kommenteista: Commentdemo1 Manage

Muut esimerkit mahdollisista kiusallisista käyttötavoista

Korjaus

Ajattelin ekaksi, että helpointa saattaisi olla kieltää style-elementti sanitoinnissa, mutta koska samaa sanitointia käytetään myös tekstilohkoissa, vaarana on, että validit style-elementit hävitetään. Toisaalta tyylithän TIMissä voi kirjoittaa dokumenttiasetuksiin, eli elementin sanitointi ei rikkoisi dokumenttejä. Toisaalta style-elementin salliminen tähän mennessä tarkoittaa, että joku olisi saattanut tehdä tyylejä liittämällä dokumenttiin juuri näitä style-lohkoja eikä TIMin dokumenttiasetuksia.

Täten kysymys: onko TIMissa tällä hetkellä yhtäkään dokumenttia, joiden sisällössä on <style>-elementtiä käytetty? Jos on, style-attribuutin sanitointi pois saattaa olla ongelma, eli dokumentit pitää korjata.

dezhidki commented 3 years ago

In GitLab by @vesal on Feb 24, 2021, 17:48

Voikos kommenteissa olla eri sanitize kun dokussa. Koska miksei dokuun saisi laittaa tyylejä kun niitä saa laittaa asetukiinkin. Toki sitten jos on keskustelutyylinen doku johon kuka vaan saa lisätä, niin sitten eri juttu. Tosin tarviiko dokussa style jos sen voi laittaa asetuksiin?

dezhidki commented 3 years ago

Voikos kommenteissa olla eri sanitize kun dokussa.

Voi olla. Mietin vaan sitä, että tarviiko välttämättä sitä dokussaikaan koska kuten mainitset:

Tosin tarviiko dokussa style jos sen voi laittaa asetuksiin?

Voi, ja näin se kannattaa tehdäkin, koska sen kautta saa myös SCSS:n käyttöön.

Teinkin ekaksi tiketin sen takia, että kaipaisin asiaan mielipidettä ja toteutettavuutta: stylen kieltäminen dokuissa ja kommentteissa olisi vain yhden rivin muutos versus n. 10 riviä + lisälogiikkaa, jolla käsittelee kommentit erikseen dokuista.

Jos tällä hetkellä on erittäin monta dokua, jossa tuota style käytetään, niin on mielestäni sitten parempi lisätä kommentteille erillinen käsittely. Jos ei (eli dokuissa käytetään "oikeaoppisesti" dokumenttiasetuksia, johon omat tyylit kirjoitellaan), niin helpompaa mielestäni kieltää elementin kokonaan ellei sille ole oikein hyvää syytä.

dezhidki commented 3 years ago

sitten jos on keskustelutyylinen doku johon kuka vaan saa lisätä, niin sitten eri juttu

Joo, sitäkään en ensin ajatellut, eli sekin sitten avaa mahdollisuuden kenen tahansa upottaa tyylejä sivulle.

dezhidki commented 3 years ago

In GitLab by @vesal on Feb 24, 2021, 18:14

Pistä TIMIN suureenuslasi-hakuun <style> niin näet dokut joissa sitä on käytetty. 68 kpl, joista suuressa osassa se on iframessa.

Samalla kyllä löytyy aika paljon juttuja joista on ruvennut tulemaan 404, esim:

https://tim.jyu.fi/view/tau/toisen-asteen-materiaalit/maol-koulutusten-kansio-opettajille/tampereen-koulutustesti/tehtavapohjaopettajille

jossa esim Abit-editori ei toimi.

Mutta nopealla vilkaisulla yksikään noista 68:sta ei sisältänyt <style> varsinaisessa TIM-koodissa.

dezhidki commented 3 years ago

Hmm, näyttää että muutamassa sivussa käytetään suoraan HTMLää, jossa style käytössä

Esimerkiksi

Eli siis ainakin kahdessa dokumentissa sille on ihan järkevää käyttöä. Näyttää myös siltä, että ainakin tuossa kurssin läpiviennissä tuota taulukkoa on copypastettu jostain työkalusta, eli se herättäisi dokumentin kirjoittajassa ihmetystä, jos copypastettu HTML-pätkä ei toimi suoraan. Käyttäjänhän pitäisi sitten jostain tietää, että tyylit lisätään sivulle eri tavoin kuin muu sisältö.

Varmaan täten järkevää kai tehdä niin, että estän style-elementin vain kommenteille. Dokulla sillä on vähän käyttöä, mutta se varmaan saisi aikaan ihmetystä, jos jonkun työkalun valmiin HTML:n kopiointi ei toimisi.

dezhidki commented 3 years ago

In GitLab by @vesal on Feb 24, 2021, 19:10

Tuoss style on iframessa ja esimerkissä (pre-tyylissä)

Eli siis ainakin kahdessa dokumentissa sille on ihan järkevää käyttöä. Näyttää myös siltä, että ainakin tuossa kurssin läpiviennissä tuota taulukkoa on copypastettu jostain työkalusta, eli se herättäisi dokumentin kirjoittajassa ihmetystä, jos copypastettu HTML-pätkä ei toimi suoraan. Käyttäjänhän pitäisi sitten jostain tietää, että tyylit lisätään sivulle eri tavoin kuin muu sisältö.

Saako sen stylen vaikutusaluetta rajattua muuten kuin iframella. Tai sitten hätätilassa tekisin jonkin lohkon attribuutin jolla voi pyytää että style saa lisätä.

Varmaan täten järkevää kai tehdä niin, että estän style-elementin vain kommenteille. Dokulla sillä on vähän käyttöä, mutta se varmaan saisi aikaan ihmetystä, jos jonkun työkalun valmiin HTML:n kopiointi ei toimisi.

No en muutenkaan (Lahosen vastavöitteistä huolimatta) kannustaisi suoran HTML käyttöä TIM tekstissä. Esim se tekee mahdottomaksi LaTeX-tulostuksen.

Ja se stylen käyttää antaisi siellä keskusteludokuissa mahdollisuuden rikkoa dokua. Eli jos muita ei löydy kuin nuo kaksi, niin kieltäisin ja korjaisin nuok kaksi käsin ja minä voi tarvittaessa infota Tonia siitä miten se pitää tehdä. Laittakoon sitten ohjeisiin kuinka tuo pitää kopioida.

Vesa

dezhidki commented 3 years ago

In GitLab by @vesal on Feb 24, 2021, 19:18

ja korjaisin nuok kaksi käsin ja minä voi tarvittaessa infota Tonia siitä miten se pitää tehdä. Laittakoon sitten ohjeisiin kuinka tuo pitää kopioida.

Infosin jo Tonia että tällainenm uutos saattaa tulla ja neuvoin miten laittaa tyylit.

dezhidki commented 3 years ago

Okei, no kiellän sitten kummatkin.

dezhidki commented 3 years ago

In GitLab by @vesal on Feb 24, 2021, 19:22

Okei, no kiellän sitten kummatkin.

Kokeile että nuo kaksi dokua korjaantuu css:llä.

dezhidki commented 3 years ago

Joo. Heti, kun TIM tulee takaisin pystyyn.

dezhidki commented 3 years ago

Kokeile että nuo kaksi dokua korjaantuu css:llä.

Tämä tehty, eli style tagien sisältö laitettu dokumentin asetuksiin. Taulukko toimii edelleen hyvin.

Saako sen stylen vaikutusaluetta rajattua muuten kuin iframella.

Ainoa, joka tulee iframen lisäksi mieleen on Shadow DOM, eli jonkun elementin sisällön eristäminen. Sen ainoa miinus on se, että se pitää kytkeä päälle elementtikohtaisesti JavaScriptilla (MDN:ssä lisätietoa Shadow DOMista).

Ja jonkun lisäattribuutin tekeminen tarkoittaisi, että esim. keskusteludokussa kuka tahansa voisi kai sen attribuutin laittaa lohkoon. Enpä nyt tiedä, miten sen fiksusti estäisi (varmaan katsomalla, kuka lohkon alun perin lisäsi?).

dezhidki commented 3 years ago

In GitLab by @vesal on Feb 24, 2021, 19:54

Ja jonkun lisäattribuutin tekeminen tarkoittaisi, että esim. keskusteludokussa kuka tahansa voisi kai sen attribuutin laittaa lohkoon. Enpä nyt tiedä, miten sen fiksusti estäisi (varmaan katsomalla, kuka lohkon alun perin lisäsi?).

Lähinnä kai se voisi olla että manage-oikeuksilla tai suuremmilla saisi käyttää style (ja mhdollisesti jotkin muutkain kiellettyä?) mutta edit-oikeuksilla ei. Mutta mennään aluksi niin että sitä ei saa kukaan käyttää. Katsotana jos jokukeksii valtavan hyvän perusteen käytölle.

Tuo manage-oiukeudellakin rajoitettu jättäisi sellaisen porsaanreiän, että voisi houkutella lainaamaan lohkon toiseen paikkaan jossa ei ole manage-oikeuksia ja saisi sen sekaisin. Eli ei tuossa taida olla ihan fiksua tapaa tehdä turvallisesti.

dezhidki commented 3 years ago

!166 nyt mergetty