[php] Javascript filteren

[WernerL]

Ik ben bezig met een functie die javascript filtert.
Dit werkt prima, enkel gaat het fout op het moment dat er enters staan.

Mijn functie:

code:

function noScript($string)
{
    //Strip <script> tags
    $match = "/<script(.*)>(.*)<\\/script>/";
    $string = preg_replace($match, "<div>test</div>", $string);
    
    //Strip links who execute javascript.
    $match = "/href=(['\"]).*?javascript:(.*)(['\"])/";
    $string = preg_replace($match, "href='#'", $string); 
    
    // strip onClick events
    $match = "/onclick=(['\"])(.*)(['\"])/";
    $string = preg_replace($match, "", $string);
    
    return $string;
}

Het gaat om de bovenste preg_replace. Deze gaat fout op het moment dat er iets als:

<script>
blaat()
</script>

staat, en dat komt omdat er enters instaan. Op het moment dat het op 1 regel staat gaat het wel goed. html moet wel mogelijk blijven dus htmlspecialchars() heeft geen zin. striptags() ook al naar gekeken maar daarmee kan ik alleen aangeven welke tags WEL mogen, en niet welke tags ik wil strippen. Het gaat nml alleen maar om de <script> tag.

iemand die me hiermee kan helpen?

[CodeCaster]

Dan schrijf je toch even een script of functie waarmee je test of je uitkomst klopt?

PHP:

// test strings
$strings[] = "<script>hoi(); doei();</script>";
$strings[] = "<script>\nblaat()\n</script>";
$strings[] = "<script>\r\nblaat()\r\n</script>";

// regexp to match
$match = "#<script(.*)>(.*)</script>#is";

$i = 1;
// loop through all test strings
foreach($strings as $string) {
  print "<b>" . $i++ . ":</b>";
  
  // print original string 
  print "<pre>" . htmlentities($string) . "\n";
  // print replaced string
  print htmlentities(preg_replace($match, "<div>test</div>", $string)) . "</pre>";
}

Kijk even naar de modifiers, dan had je gezien dat de s-modifier is wat je zoekt:

If this modifier is set, a dot metacharacter in the pattern matches all characters, including newlines.

Ik heb ook de i-modifier gebruikt om hem case-insensitive te maken. En ik zou hem trouwens ook ungreedy maken, maar hoe en waarom mag je zelf opzoeken om toch nog wát te leren van deze voorgekauwde post

[ Voor 9% gewijzigd door CodeCaster op 07-04-2009 12:31 ]

[Woy]

Als je opzoekt wat de . betekent zul je zien dat het betekent.

Matches any single character except line break characters \r and \n. Most regex flavors have an option to make the dot match line break characters too.

Je matched dus precies wat je aangeeft.

Met modifiers kun je wel aangeven dat de . ook multiline moet matchen

WernerL schreef op dinsdag 07 april 2009 @ 12:07:
striptags() ook al naar gekeken maar daarmee kan ik alleen aangeven welke tags WEL mogen, en niet welke tags ik wil strippen. Het gaat nml alleen maar om de <script> tag.

iemand die me hiermee kan helpen?

Je kan eventueel toch gewoon een lijst met alle tags maken, en daar de script tag tussenuit halen.

[ Voor 12% gewijzigd door Woy op 07-04-2009 12:31 ]

[Verwijderd]

Doe eens iets in de zin van:

$match = "/bla/is";

Dan is hij case-insensative en gaat hij over meerdere regels (dat "is")

[WernerL]

Ik had inderdaad gevonden dat de . geen newlines selecteerd maar ik had nog niet gevonden dat je dat wel voor elkaar kunt krijgen met de s modifier..
Hij werkt nu, jeuj !

Thanks mensen. :-)

[Erkens]

Beetje offtopic, maar vergeet niet dat er meer mogelijkheden zijn om javascript te gebruiken, je hebt bijvoorbeeld al de onclick handler gechecked. Maar er zijn veel meer "event handlers", wat dacht je van de muis en toetsenbord events om er maar een paar te noemen.

[Janoz]

Naast alle on* attributen waar javascript in kan staan heb je natuurlijk ook nog de fantastische behaviours die je voor IE in je style kunt zetten.


Uiteindelijk zul je altijd wat vergeten. Het is daarom beter om niet met een blakclist, maar met een whitelist te gaan werken.

[T-MOB]

Erkens schreef op dinsdag 07 april 2009 @ 12:35:
Beetje offtopic, maar vergeet niet dat er meer mogelijkheden zijn om javascript te gebruiken, je hebt bijvoorbeeld al de onclick handler gechecked. Maar er zijn veel meer "event handlers", wat dacht je van de muis en toetsenbord events om er maar een paar te noemen.

Gevaarlijker nog zijn de events waarvan je zeker weet dat ze getriggered gaan worden: onload/onerror. Wat dat betreft denk ik dat de gekozen aanpak niet veilig te krijgen is. Wanneer je alle events hebt afgevangen dan moet je nog in alle browsers controleren hoe er wordt omgegaan met malformed html. In Firefox kom je op deze manier bijvoorbeeld al langs de test:

HTML:

<script type="text/javascript">
alert('boo');
</script >

Persoonlijk zou ik daarom eigenlijk altijd voor een ubb/rml-achtige oplossing gaan als het gaat om bezoekers die content mogen intypen.

[WernerL]

Nja, het is niet voor mijn eigen project. De baas wil dat html mogelijk is. Zelf zou ik ook liever voor een ubb parser gaan ja.

[Erkens]

Janoz schreef op dinsdag 07 april 2009 @ 12:42:
Naast alle on* attributen waar javascript in kan staan heb je natuurlijk ook nog de fantastische behaviours die je voor IE in je style kunt zetten.


Uiteindelijk zul je altijd wat vergeten. Het is daarom beter om niet met een blakclist, maar met een whitelist te gaan werken.

En ook met een whitelist moet je opletten dat je bijvoorbeeld geen urls accepteert die ook nog eens javascript kunnen bevatten

[Kalentum]

Ik gebruik nu html purifier

HTML Purifier is a standards-compliant HTML filter library written in PHP. HTML Purifier will not only remove all malicious code (better known as XSS) with a thoroughly audited, secure yet permissive whitelist, it will also make sure your documents are standards compliant, something only achievable with a comprehensive knowledge of W3C's specifications.

Scheelt een hoop werk.

[mocean]

Deze is ook leuk:

code:

<scri<script>foo();</script>pt>
evilCode();
</scri<script>foo();</script>pt>

Je moet dus sowieso die noScript functie herhalen!

[Erkens]

mocean schreef op dinsdag 07 april 2009 @ 13:38:
Deze is ook leuk:

code:

<scri<script>foo();</script>pt>
evilCode();
</scri<script>foo();</script>pt>

Je moet dus sowieso die noScript functie herhalen!

Dan moet je die blijven herhalen tot je geen script meer tegenkomt, niet heel erg efficient Beter is het om het script niet door "" te replacen, maar door een html comment oid, dan heb je dat "probleem" ook niet

[HuHu]

mocean schreef op dinsdag 07 april 2009 @ 13:38:
Deze is ook leuk:

code:

<scri<script>foo();</script>pt>
evilCode();
</scri<script>foo();</script>pt>

Je moet dus sowieso die noScript functie herhalen!

Om XSS tegen te gaan moet je gewoon de <, > en " vervangen voor &lt;, &gt; en &quot;. Dat is voldoende. Of gebruik htmlspecialchars() of htmlentities() of een speciale escape() methode uit een PHP framework.

[RobIII]

HuHu schreef op dinsdag 07 april 2009 @ 14:26:
[...]

Om XSS tegen te gaan moet je gewoon de <, > en " vervangen voor &lt;, &gt; en &quot;. Dat is voldoende. Of gebruik htmlspecialchars() of htmlentities() of een speciale escape() methode uit een PHP framework.

En je hebt het topic ook gelezen?

[HuHu]

RobIII schreef op dinsdag 07 april 2009 @ 14:33:
[...]

En je hebt het topic ook gelezen?

Nee.

Maar ik heb 'm nu wel even gelezen. Uiteraard blijft mijn opmerking nog steeds geldig, alleen heeft 't weinig te doen met dit topic . Al is een goede tip nooit weg natuurlijk.

Waar WernerL ook gebruik van zou kunnen maken, als alternatief voor de HTML purifier, is iets als FCKeditor of TinyMCE. Daarmee kunnen gebruikers ook HTML plaatsen, maar wordt het ook gefilterd op 'gevaarlijke' elementen.

[RobIII]

HuHu schreef op dinsdag 07 april 2009 @ 14:51:
maar wordt het ook gefilterd op 'gevaarlijke' elementen.

Die ik vervolgens gewoon submit buiten FCK/TinyMCe om
Als je het over goede tips hebt: deze controle (slash opschoonactie) wil je server-side in handen houden Wat we dus aan 't bespreken waren in dit topic

[ Voor 25% gewijzigd door RobIII op 07-04-2009 14:56 ]

[Cartman!]

Is het geen beter idee om je baas in te lichten welke gevaren het kan veroorzaken als ie html wil toestaan? Spreek anders af dat je enkel bepaalde tags gaat toestaan (<strong><em><a>) of inderdaad overstapt op UBB. Alle XSSs ondervangen gaat denk ik een stuk langer duren en serieus, welke gemiddelde gebruiker gaat html gebruiken? UBB is vaak net zo duidelijk.

[Janoz]

WernerL schreef op dinsdag 07 april 2009 @ 13:18:
Nja, het is niet voor mijn eigen project. De baas wil dat html mogelijk is. Zelf zou ik ook liever voor een ubb parser gaan ja.

Dus? Dan zorg je toch dat html mogelijk is?

Van mijn medewerkers zou ik verwachten dat ze een bepaalde zelfredzaamheid hebben dat ze daar zelf wel wat mee kunnen. Ik kan me namelijk niet voorstellen dat je baas heeft gezegd 'dat alle html mogelijk is'. In een dergelijke situatie leg ik hem eerst de consequenties uit en vervolgens vertel ik hem wat je met whitelisting kunt doen. Er zijn meer oplossingen dan alleen UBB. Er is zelfs een php functie die whitelisting ondersteunt.

Erkens schreef op dinsdag 07 april 2009 @ 13:21:
[...]

En ook met een whitelist moet je opletten dat je bijvoorbeeld geen urls accepteert die ook nog eens javascript kunnen bevatten

Klopt. Whitelisting zul je moeten doorvoeren op de tags, de attributen die de tags mogen hebben en de waarde van die attributen.

[ Voor 20% gewijzigd door Janoz op 07-04-2009 15:12 ]

[DexterDee]

Alhoewel ik het eens ben met bovenstaande posters over het whitelisten van 'goede' tags in plaats van blacklisten van 'foute' tags wil ik toch nog een stukje script delen.

Onderstaande code is een Smarty plugin die ik een tijd terug heb gemaakt om gemakkelijk richtext contents veilig(er) te maken tegen cross-side scripting. De functionaliteit is erg simpel losstaand te gebruiken, bijvoorbeeld voor doeleinden van de TS.

Voor mensen die dit in Smarty zouden willen gebruiken, copy/paste deze code in de Smarty/plugins directory onder genoemde filename en gebruik het op velden met de xss modifier, e.g.: {$htmltekst|xss}

Hopelijk komt het iemand nog van pas. Het script is uitgebreid getest om te werken met HTML output van WYSIWYG HTML editors en draait al enige tijd zonder problemen live. Gezien de mogelijkheden en output van moderne HTML editors is het whitelisten een zeer moeilijk c.q. tijdrovend klusje. Vandaar deze invalshoek.

DISCLAIMER: Whitelisten is altijd beter. Hoewel dit script vrij uitgebreid is, kan het zijn dat er toch nog mogelijkheden zijn voor een aanval die dit script niet dekt.

PHP:

<?php
/*
* Smarty plugin
* ---------------------------------------------------------------------
* File:     modifier.xss.php
* Type:     modifier
* Name:     xss
* Author:   Christian Stocker <[e-mail]>
* Author:   Kohana Team
* Author:   DexterDee
* Purpose:  escapes the most commonly used cross site scripting attacks
* ---------------------------------------------------------------------
*/
function smarty_modifier_xss($string)
{
        if (get_magic_quotes_gpc()) {
                $string = stripslashes($string);
        }
        $string = str_replace(array("&amp;","&lt;","&gt;"),array("&amp;amp;","&amp;lt;","&amp;gt;"),$string);
        // fix &entitiy\n;
        $string = preg_replace('#(&\#*\w+)[\x00-\x20]+;#u',"$1;",$string);
        $string = preg_replace('#(&\#x*)([0-9A-F]+);*#iu',"$1$2;",$string);
        $string = html_entity_decode($string, ENT_COMPAT, "UTF-8");
        
        // remove any attribute starting with "on" or xmlns
        $string = preg_replace('#(<[^>]+[\x00-\x20\"\'\/])(on|xmlns)[^>]*>#iUu', "$1>", $string);
        
        // remove javascript: and vbscript: protocol
        $string = preg_replace('#([a-z]*)[\x00-\x20\/]*=[\x00-\x20\/]*([\`\'\"]*)[\x00-\x20\/]*j[\x00-\x20]*a[\x00-\x20]*v[\x00-\x20]*a[\x00-\x20]*s[\x00-\x20]*c[\x00-\x20]*r[\x00-\x20]*i[\x00-\x20]*p[\x00-\x20]*t[\x00-\x20]*:#iUu', '$1=$2nojavascript...', $string);
        $string = preg_replace('#([a-z]*)[\x00-\x20\/]*=[\x00-\x20\/]*([\`\'\"]*)[\x00-\x20\/]*v[\x00-\x20]*b[\x00-\x20]*s[\x00-\x20]*c[\x00-\x20]*r[\x00-\x20]*i[\x00-\x20]*p[\x00-\x20]*t[\x00-\x20]*:#iUu', '$1=$2novbscript...', $string);
        $string = preg_replace('#([a-z]*)[\x00-\x20\/]*=[\x00-\x20\/]*([\`\'\"]*)[\x00-\x20\/]*-moz-binding[\x00-\x20]*:#Uu', '$1=$2nomozbinding...', $string);
        $string = preg_replace('#([a-z]*)[\x00-\x20\/]*=[\x00-\x20\/]*([\`\'\"]*)[\x00-\x20\/]*data[\x00-\x20]*:#Uu', '$1=$2nodata...', $string);
        
        //remove any style attributes, IE allows too much stupid things in them, eg.
        //<span style="width: expression(alert('Ping!'));"></span> 
        // and in general you really don't want style declarations in your UGC

        $string = preg_replace('#(<[^>]+[\x00-\x20\"\'\/])style[^>]*>#iUu', "$1>", $string);
        
        //remove namespaced elements (we do not need them...)
        $string = preg_replace('#</*\w+:\w[^>]*>#i',"",$string);

        //remove really unwanted tags
        do {
                $oldstring = $string;
                $string = = preg_replace('#</*(?:applet|b(?:ase|gsound|link)|embed|frame(?:set)?|i(?:frame|layer)|l(?:ayer|ink)|meta|object|s(?:cript|tyle)|title|xml)[^>]*+>#i', '', $string);
        } while ($oldstring != $string);

        return $string;
}

[ Voor 14% gewijzigd door DexterDee op 07-04-2009 20:00 ]

[RobIII]

DexterDee schreef op dinsdag 07 april 2009 @ 19:12:
Onderstaande code is een Smarty plugin die ik een tijd terug heb gemaakt

Grappig; ik kom diezelfde code op diverse plaatsen met een ander kopje tegen

http://svn.bitflux.ch/rep...classes/externalinput.php
http://dev.kohanaphp.com/...raries/Input.php?rev=3219
http://fisheye.liip.ch/br...externalinput.php?r=trunk

[ Voor 255% gewijzigd door RobIII op 07-04-2009 19:32 ]

[Little Penguin]

Janoz schreef op dinsdag 07 april 2009 @ 15:10:
[...]
Dus? Dan zorg je toch dat html mogelijk is?
[...]

Precies. Er staan volgens mij al voldoende oplossingen in het topic, maar toch wil ik een oplossing die ik ooit gebruikt heb de (mede)lezers niet onthouden:

Ik verving toen gewoon alle < door &lt; en > door &gt;

OK en dan?

Daarna ben ik doelgericht de HTML-tags die doorgelaten moesten worden (en dat waren er niet zo veel) weer terug gaan veranderen naar een HTML-tag. Dus &lt;strong&gt; werd dan weer <strong> en zo verder.

Voor die toepassing was het ruim voldoende (alleen de b,i,u elementen werden gevraagd), in hoeverre het hier ook voldoende is dat vraag ik me af. Zeker omdat deze oplossing geen attributen op de elementen toelaat. Aan de andere kant is het wel lekker veilig en als er slechts een beperkte set aan HTML-elementen ondersteund hoeft te worden kan het ook nog eens gemakkelijk te implementeren...

Maar goed, er zijn al voldoende andere oplossingen aangedragen in eerdere reacties :-)

[ Voor 0% gewijzigd door Little Penguin op 07-04-2009 19:27 . Reden: HTML Entities (& amp ; et al) ]

[RobIII]

Little Penguin schreef op dinsdag 07 april 2009 @ 19:25:
Ik verving toen gewoon alle < door &lt; en > door &gt;

OK en dan?

Daarna ben ik doelgericht de HTML-tags die doorgelaten moesten worden (en dat waren er niet zo veel) weer terug gaan veranderen naar een HTML-tag. Dus &lt;strong&gt; werd dan weer <strong> en zo verder.

En hoe verschilt dat met whitelisten

Little Penguin schreef op dinsdag 07 april 2009 @ 19:25:
Zeker omdat deze oplossing geen attributen op de elementen toelaat.

Waarom niet? Kan prima hoor?

[ Voor 19% gewijzigd door RobIII op 07-04-2009 19:29 ]

[DexterDee]

RobIII schreef op dinsdag 07 april 2009 @ 19:18:
<hier stond onzin>

Ik was al bezig om inhoudelijk te reageren op je reply, toen 'ie in een keer weg was. Die control characters worden door sommige (oudere) browsers echt genegeerd en dan is je source ineens iets heel anders dan wat er uitgevoerd wordt. Wat betreft tags als base, die filter ik gewoon eruit omdat ze irritant kunnen zijn. Zo verplaats je je relatieve pad met de <base> tag. Om maar niet te spreken over bgsound of blink

[RobIII]

DexterDee schreef op dinsdag 07 april 2009 @ 19:29:
[...]

Ik was al bezig om inhoudelijk te reageren op je reply, toen 'ie in een keer weg was.

Er staat nu weer wel-zin

Anyhow; ik snapte de 0x00-0x20 niet zo zeer; maar het gaat dan natuurlijk voornamelijk om tabs (en evt. spaties). Dat drong pas later door En die base snapte ik niet omdat 't (AFAIK) niet gevaarlijk is in XSS context, hoewel ik me daar gevoelsmatig nog wel wat bij voor kan stellen. Maar die gaat er dus uit omdat 'ie "irritant" is

[ Voor 36% gewijzigd door RobIII op 07-04-2009 19:37 ]

[Woy]

Little Penguin schreef op dinsdag 07 april 2009 @ 19:25:
[...]
Ik verving toen gewoon alle < door &lt; en > door &gt;

OK en dan?

Daarna ben ik doelgericht de HTML-tags die doorgelaten moesten worden (en dat waren er niet zo veel) weer terug gaan veranderen naar een HTML-tag. Dus &lt;strong&gt; werd dan weer <strong> en zo verder.

En wat dan als ik in mijn post &lt;strong&gt; type? dan word dat ten onrechte veranderd in een <strong>

[Little Penguin]

RobIII schreef op dinsdag 07 april 2009 @ 19:28:
[...]

En hoe verschilt dat met whitelisten

Niet eigenlijk hè - als ik 't zo terug lees

[...]

Waarom niet? Kan prima hoor?

Stel je voor dat ik <strong style="color:-kleurtje-;"> wil gebruiken, dan gaat het terugvervangen van de &lt;strong style="color:-kleurtje-;"&gt; wat lastig als je alleen controleert op &lt;strong&ampgt; - je moet dan een wat ingewikkeldere functie schrijven die rekening houdt met de mogelijke attributen.

Of je moet alle mogelijke attributen toelaten, maar dat kunnen er best veel worden en als je dat via een hardcoded vervang gaat doen...

Zoals ik al aangegeven hebt was het ook alleen maar om de i,b,u elementen toe te laten. Dat kun je wel doen met "dom" vervangen, maar als het ingewikkelder wordt is het niet direct praktisch om te vervangen zoals hierboven omschreven...

[DexterDee]

RobIII schreef op dinsdag 07 april 2009 @ 19:31:
[...]

Er staat nu weer wel-zin

Grappig, het basisgedeelte van die code heb ik waarschijnlijk van dezelfde plaats als waar deze "Christian Stocker" hem vandaan heeft gehaald. Die stond op een website over XSS en PHP. Er stond toen nog geen licentieheader boven en bestond uit niet meer dan een aantal regeltjes in een blogpost. Goed om te zien dat die kohana jongens er nog een aantal dingen aan hebben verbeterd.

edit: Ik heb de nieuwe features even gemerged in mijn versie van het script en de authors toegevoegd

[ Voor 8% gewijzigd door DexterDee op 07-04-2009 20:02 ]

[Little Penguin]

Woy schreef op dinsdag 07 april 2009 @ 19:41:
[...]

En wat dan als ik in mijn post &lt;strong&gt; type? dan word dat ten onrechte veranderd in een <strong>

Inderdaad, moet ik er wel bij zetten dat & ook opgevangen werd - tenminste, als ik 't me goed herinner. De code waar ik aan refereer heb ik ruim 5 jaar geleden eens in elkaar geflanst...

Voor zover het 't me bijstaat werd eerst & door &amp; vervangen en daarna de eerder genoemde truc uitgehaald - ook hier was 't zo dat de klant zo graag die 3 elementen wilde kunnen gebruiken. Ondersteuning voor andere HTML-entities was niet aanwezig (en ook niet nodig, was gewoon Latin1)

[ Voor 1% gewijzigd door Little Penguin op 07-04-2009 19:53 . Reden: nohtml... ]

[Gomez12]

Tja gevoelsmatig kies ik toch liever voor ubb dan voor een gewhiteliste html.
Met gewhiteliste html heb ik in een ver verleden al vaak genoeg meegemaakt dat iemand met meer html-ervaring er enkele tags bijwilde, en omdat dit dan vlug vlug moest ( de persoon had meestal de html al kant en klaar staan en zat gewoon te wachten tot hij het kon posten ).

Met UBB tags heb ik altijd een simpele discussie ( de tag is er niet en moet eerst geprogrammeerd worden, dat kost tijd en moet getest worden ) terwijl ik met gewhiteliste html codes een moeilijkere discussie heb ( jullie filteren het weg, haal het filter even weg en dan kan ik mijn gang gaan )
En dan kies ik bij een klantdiscussie toch liever voor de makkelijker te verantwoorden weg...