[php/js]Hernoemen onclick en XSS

[Tjolk]

Voor een IPB 3.0.x forum ben ik bezig LaTeX te implementeren. Onderdeel daarvan is dat men op de gecreëerde code kan klikken en er dan een js-popupje verschijnt die laat zien wat de onderliggende LaTeX code is. Bijvoorbeeld:

HTML:

<!--LaTeX_HtmlStart: CgomIzA5MjtpbnRfMF57JiMwOTI7aW5mdHl9JiMwOTI7ZnJhY3t4XjN9e2VeeC0xfSBkeAoK :LaTeX_HtmlStart--><a style="cursor:pointer;" onclick="LatexPopup('<br>\\int_0^{\\infty}\\frac{x^3}{e^x-1} dx<br>',0)"><img src='/pad/naar/pictures/ce55f5480751b1e657f54ddb078dbf1b.gif' title='LaTeX' alt='LaTeX' border=0 align='absmiddle'></a><!--LaTeX_HtmlEnd-->

Tenminste, dat zou het moeten zijn. IPB 3.0 kent namelijk een public function in de BBcode parser die onlicks e.d. aanpast. Daarvan zie het relevante deel er als volgt uit:

PHP:

        $event_handlers = array( 'mouseover', 'mouseout', 'mouseup', 'mousemove', 'mousedown', 'mouseenter', 'mouseleave', 'mousewheel',
                                 'contextmenu', 'click', 'dblclick', 'load', 'unload', 'submit', 'blur', 'focus', 'resize', 'scroll',
                                 'change', 'reset', 'select', 'selectionchange', 'selectstart', 'start', 'stop', 'keydown', 'keyup',
                                 'keypress', 'abort', 'error', 'dragdrop', 'move', 'moveend', 'movestart', 'activate', 'afterprint',
                                 'afterupdate', 'beforeactivate', 'beforecopy', 'beforecut', 'beforedeactivate', 'beforeeditfocus',
                                 'beforepaste', 'beforeprint', 'beforeunload', 'begin', 'bounce', 'cellchange', 'controlselect',
                                 'copy', 'cut', 'paste', 'dataavailable', 'datasetchanged', 'datasetcomplete', 'deactivate', 'drag',
                                 'dragend', 'dragleave', 'dragenter', 'dragover', 'drop', 'end', 'errorupdate', 'filterchange', 'finish',
                                 'focusin', 'focusout', 'help', 'layoutcomplete', 'losecapture', 'mediacomplete', 'mediaerror', 'outofsync',
                                 'pause', 'propertychange', 'progress', 'readystatechange', 'repeat', 'resizeend', 'resizestart', 'resume',
                                 'reverse', 'rowsenter', 'rowexit', 'rowdelete', 'rowinserted', 'seek', 'syncrestored', 'timeerror',
                                 'trackchange', 'urlflip',
                                );
        
        foreach( $event_handlers as $handler )
        {
            $txt = str_ireplace( 'on' . $handler, 'on' . $handler, $txt );
        }

Ofwel: mijn onclick wordt onklaar gemaakt.Daarnaast kent de functie nog wat replaces waarin dingen als "javascript", "window", "<script>" e.d. worden hernoemd.

edit: de 2e "on" in de str_ireplace is een &# 111; maar die wordt hier geparsed.

Nu heb ik drie vragen:
1. Is dit nu een fatsoenlijke manier om XSS te voorkomen?
2. Zou het heel veel risico met zich meebrengen om de onclick uit de array te halen?
3. Indien dat wel risico's met zich meebrengt, heeft iemand een ander geniaal idee?

[Precision]

Als ik met een onclick aan de slag kan zou ik het zo kunnen maken dat als iemand er op klikt hij doorgestuurd wordt naar een pagina met als parameter <script>document.cookie</script> www.mijnsite.ext/?paramet...>document.cookie</script> Dan heb ik dus iemand zijn cookies gestolen, met een beetje geluk zit er een session id bij of gwn een login hash en kan ik dus de login overnemen. PS. Dit is gewoon een voorbeeld van wat er mis kan gaan, ik vermoed dat mijn voorbeeld afgehandeld wordt maar als je maar lang genoeg zoekt vind je altijd wel iets.

[_js_]

1. Het is niet een fatsoenlijke manier om javascriptinjectie te voorkomen, omdat als er nu nog geen browsers zijn die nog meer eventhandlers ondersteunen, dan komen die er in de toekomst wel en zit je ineens met een gat in je beveiliging.

2/3. Zonder de rest van de code te kennen is het niet te zeggen wat het risico is van het weghalen van een handler.

Wat je eventueel kunt doen is jouw LaTeX ding pas invoegen nadat die replace is gedaan.

[Verwijderd]

Het stukje code uit de topicstart bevat geen enkele beveiliging. Het enige dat gebeurt is dat hoofdletters in onclick etc. worden vervangen door onderkasten.

[Voutloos]

@Cheatah, dat dacht ik eerst ook, maar de 'o' in de replace staat er als html entity.

De opzet van deze beveiliging is redelijk stupide. Door te werken met een blacklist ben je niet voorbereid op nieuwe features in de toekomst, en bovendien ontgaat het nut van de subtiele replace mij: beuk het er dan gewoon helemaal uit. Lekker nuttig om het er met een minimale verminking alsnog in te laten.

[Verwijderd]

Aha, nu zie ik het ook ja. Wat een gore manier. Het is mij ook een raadsel waarom de boel niet door een fatsoenlijke parser wordt gehaald.

[Data-base]

Ik ken de parser van IPB niet, maar het is een rare manier om xss te voorkomen. Sowieso is het beter om te whitelisten dan om te blacklisten.

Wat is overigens raar vind is dat de html nog door de functie gaat die eventhandler "escaped", het is wel een makkelijke manier om zeker te weten dat de woorden die daar staan nooit zo worden ge-output, maar het is totaal geen flexibele manier.

Je moet kijken of je je code can invoegen nadat de o's zijn gefilterd, zo niet dan kan je nog meer bij gaan hacken en het volgende doen:

gebruik als naam van de eventhandler MijnSuperVetteEventhandler en zorg dat dit na het paseren van de "parser" MijnSuperVetteEventhandler wordt vervangen door onclick.
Om te voorkomen dat iemand anders dit ook kan doen, kan je een salted hash maken van de inhoud van je eventhandler en deze checken voordat je output. Zo zit de security van je systeem in de salt van je hash.

Ja ik weet het, het is heel smerig )

[Tjolk]

Dat het een gore manier is, daar ben ik het volledig mee eens. Maar goed, het werkt inderdaad...

Ben er ook niet kapot van, ik zou van die gasten toch wel iets meer verwachten. Maar goed, feit is dat ik er mee om moet gaan. Wat ik natuurlijk wel kan doen is checken of die onclick tussen de LaTeX comments staat. Indien dat het geval is --> onclick toelaten; indien dat niet het geval is --> onclick weghalen.