[Forumgame] Vind de kwetsbaarheid!

[Verwijderd]

---

Zoek De Kwetsbaarheid

Na toestemming gekregen te hebben van onze lieve modjes, word hier een nieuw forum-game gestart omtrent programming.
Om dit goed te laten verlopen, hier eerst de volledige uitleg over wat de bedoeling is, wat niet de bedoeling is, wat de regels zijn,..

1. De Bedoeling

De bedoeling van het spel is, zoals de titel al zegt, kwetsbaarheden zoeken. Omdat dit topic in het programming forum staat, lijkt het me logisch dat we kwetsbaarheden in programmacode gaan zoeken.
Het gaat alsvolgt:

• Ik begin met het plaatsen van een stukje programmacode/code snippet
• Jullie zoeken uit hoe je dat stukje code kan exploiten. Natuurlijk ook uitleggen hoe je het exploit, dat levert een goede leerervaring op voor anderen.
• Degene die het juist raadt plaatst het volgende stukje code.
• Zo gaan we eenvoudig genoeg verder.
• Indien er na 1 week nog niemand de exploit gevonden heeft, mag er een andere persoon een nieuwe snippet plaatsen.

Natuurlijk horen er bij een forum-game ook regels:

2. Regels & Afspraken

De code snippets

• Het is niet de bedoeling dat dit een 'check my code'-topic wordt. Je moet dus reeds zelf weten waar de kwetbaarheid zich bevind en hoe deze te exploiten alvorens een stukje code te plaatsen.
• De rest van de code moet volledig werken. Dit omdat sommigen misschien zelf de code eens willen proberen om de exploit te zoeken + eventuele achterhouding van informatie te vermijden.
• Er mag maar 1 code snippet per keer geplaatst worden, en dit alleen door degene die de vorige kwetbaarheid als eerste gevonden had.
• De kwetsbaarheid moet toe te passen zijn zonder de code zelf aan te passen/reverse engineering/hex editing. Dit zou het maar al te gemakkelijk maken om zelf een bug te maken.
• Elke programmeertaal is toegestaan; zo krijgt iedereen de kans om mee te doen.
• Indien je zelf geen code hebt voor de anderen doe je best niet mee; zo loopt het spel niet vast.

Antwoorden
Halve exploits tellen niet. Als je wel weet waar de fout zich bevind, maar niet hoe je deze moet misbruiken telt het antwoord niet.
Zulke dingen houd je dus best voor jezelf, om het de anderen niet te gemakkelijk te maken.
Het enige geldige antwoord is de plaats waar de fout zich bevind + hoe deze te misbruiken.

3. Voorbeeld

Om het een beetje duidelijker te maken voor degenen die het nog niet helemaal snappen; hier een klein (dom) voorbeeldje.

Er wordt een code-snippet geplaatst:

PHP:

<?php 
header('Location: '.$_POST['url']); 
?>

Voor dit stukje code zou het juiste antwoord zijn:
(Omdat het hier maar 1 regeltje code was; moet er niet expliciet vermeld worden waar de fout zich bevind)

quote: user

Iedereen kan zo zijn eigen Headers toevoegen:

PHP:

$_POST="www.url.com\nHeader: Value";

Het is natuurlijk niet slim om met POST een header te maken.. Maar het is ook maar een voorbeeld.


Zo; dat was de uitleg zo een beetje.
Ik hoop natuurlijk dat jullie je hier mee gaan amuseren & eventueel nog dingen uit leert er beveiliging van je code!

---

Om het topic te starten plaats ik hier dus het eerste stukje code. Het zou een deel kunnen zijn van een bepaalde functie op een forum waar users zichzelf kunnen op aanmelden.

PHP:

<?php 
$result = mysql_query('SELECT name FROM players WHERE id=1'); 
if (!$result) { 
    die('Could not query:' . mysql_error()); 
} 
$name = mysql_result($result, 0); 

mysql_query("UPDATE players SET name='$name (banned)' WHERE name='$name'"); 
?>

Succes & veel plezier met exploiten!

[ Voor 4% gewijzigd door een moderator op 16-05-2008 16:34 . Reden: Ook hier de groetjes weggehaald ;) ]

[.oisyn]

Als je geen zin hebt om een nieuw voorbeeld te verzinnen kun je dus beter ook maar niet meeraden?

[riezebosch]

.oisyn schreef op vrijdag 16 mei 2008 @ 16:13:
Als je geen zin hebt om een nieuw voorbeeld te verzinnen kun je dus beter ook maar niet meeraden?

Is misschien wel zo handig om het topic levend te houden... Als je echt geen zin hebt kan je natuurlijk iets heel simpels doen waardoor de volgende snel het stokje weer kan overnemen? Aan de andere kant moet ik wel toegeven dat dit de drempel verhoogd om mee te doen. Want ik heb zo 1-2-3 ook geen nieuw voorbeeld klaar terwijl de huidige opdracht misschien wel met "SQL Injection" heeft te maken...

[ Voor 21% gewijzigd door riezebosch op 16-05-2008 16:19 ]

[AtleX]

Je eerste voorbeeld is niet lek, als je maar PHP 4.4.2 of 5.1.iets of later gebruikt. Daarin is header() gefixt zodat je maar 1 header tegelijk kunt versturen.

[Verwijderd]

.oisyn schreef op vrijdag 16 mei 2008 @ 16:13:
Als je geen zin hebt om een nieuw voorbeeld te verzinnen kun je dus beter ook maar niet meeraden?

Heb het even toegevoegd.
Als je nog geen snippet hebt; kan je toch snel een voorbeeldje in elkaar flansen.

[ Voor 3% gewijzigd door een moderator op 16-05-2008 16:32 . Reden: Die groeten hoeft echt niet hoor ;) ]

[Bosmonster]

Je voorbeeldcode is nogal wazig. Met het stuk op zich is niks mis volgens mij, het hangt allemaal af van wat je zou doen met de rest van de applicatie (de registratie). Maar dat laat je er niet bij zien.

De enige exploit mogelijkheid die ik zie is via die $name, maar die haal je uit de DB, dus om dat te misbruiken zul je je exploitcode bij de registratie al in de DB moeten zien te krijgen. Als je checks daar goed zijn hoeft deze code dus niet onveilig te zijn?

Zo gaat dit topic wel even duren

[ Voor 53% gewijzigd door Bosmonster op 16-05-2008 16:28 ]

[Verwijderd]

Bosmonster schreef op vrijdag 16 mei 2008 @ 16:27:
Je voorbeeldcode is nogal wazig. Met het stuk op zich is niks mis, het hangt allemaal af van wat je zou doen met de rest van de applicatie (de registratie).

Alhoewel het bannen op basis van een naam natuurlijk nogal discutabel is
Maar dat is inderdaad niet direct een exploit, eerder gewoon bad design.

[Voutloos]

Bosmonster schreef op vrijdag 16 mei 2008 @ 16:27:
Je voorbeeldcode is nogal wazig. Met het stuk op zich is niks mis, het hangt allemaal af van wat je zou doen met de rest van de applicatie (de registratie).

Nee, je moet gewoon altijd escapen, dan hoef je op dat punt ook niet na te denken of iemand nou wel of niet quotes in z'n naam mocht hebben.

[Bosmonster]

Verwijderd schreef op vrijdag 16 mei 2008 @ 16:28:
[...]

Alhoewel het bannen op basis van een naam natuurlijk nogal discutabel is
Maar dat is inderdaad niet direct een exploit, eerder gewoon bad design.

Ja ok, die code is lelijk en onlogisch natuurlijk, maar daar ging het toch niet om (Heb mn reply nog maar iets uitgebreid)

@Voutloos: ook daar heb je gelijk in natuurlijk en dat snap ik ook wel, maar wederom was dat ook niet het onderwerp van het topic als je er niets (of niet zeker) iets mee kan.

[ Voor 18% gewijzigd door Bosmonster op 16-05-2008 16:30 ]

[Verwijderd]

Voutloos schreef op vrijdag 16 mei 2008 @ 16:28:

Nee, je moet gewoon altijd escapen, dan hoef je op dat punt ook niet na te denken of iemand nou wel of niet quotes in z'n naam mocht hebben.

True, maar dan kun je net zo goed stellen dat de fout is om direct mysql_* functies aan te roepen, in plaats van de boel te wrappen in code die SQL injection onmogelijk maakt

[Bosmonster]

Verwijderd schreef op vrijdag 16 mei 2008 @ 16:30:
[...]

True, maar dan kun je net zo goed stellen dat de fout is om direct mysql_* functies aan te roepen, in plaats van de boel te wrappen in code die SQL injection onmogelijk maakt

Of het resultaat van een mysql_result direct in een query gebruiken zonder dat je eerst kijkt of de resultset wel inhoud heeft... Want zonder inhoud geeft die false terug.. en dat kan dan wat onverwachte resultaten gaan opleveren in die volgende query

M.a.w. "kwetsbaarheden" genoeg in dat kleine stukje code, maar volgens mij niet zoals TS bedoeld heeft

[ Voor 20% gewijzigd door Bosmonster op 16-05-2008 16:34 ]

[Janoz]

Ik vermoed dat het in het voorbeeld stukje toch echt gaat over een injection attack middels het kiezen van een handige username bij registratie. Vervolgens hoef je alleen nog maar een beetje te rellen en iedereen is gebanned en van naam veranderd.

[SchizoDuckie]

kortom:

registreer je op het forum met als naam

code:

 ' or '1' = '1

[IWriteCode]

Mja, id = 1, dat is meestal de id van de admin die als eerste geregistreerd heeft... kleine kans dat die a> geband wordt, en b> dat die een username heeft die de boel verpest...

Ook lijkt het me sterk, dat als je een username ziet met iets van TRUNC TABLE players, dat je die dan zo gaat bannen...

[Dido]

SchizoDuckie schreef op vrijdag 16 mei 2008 @ 17:02:
kortom:

registreer je als allereerste userop het forum met als naam

code:

 ' or '1' = '1

Toevoeging van mijn kant. Het is vrij essentieel dat je userid 1 hebt.

De code is ook alleen maar te gebruiken om de "Femme" van het betreffende forum te bannen, voor gewone users zal je andere code moeten hebben

[SchizoDuckie]

Dido schreef op vrijdag 16 mei 2008 @ 17:07:
[...]

Toevoeging van mijn kant. Het is vrij essentieel dat je userid 1 hebt.

De code is ook alleen maar te gebruiken om de "Femme" van het betreffende forum te bannen, voor gewone users zal je andere code moeten hebben

True, maar dat is even inherent aan het voorbeeld hier, al heb ik het idee dat de TS díe optie zelf nog niet door had

[ Voor 7% gewijzigd door SchizoDuckie op 16-05-2008 17:12 ]

[Bosmonster]

SchizoDuckie schreef op vrijdag 16 mei 2008 @ 17:10:
[...]

True, maar dat is even inherent aan het voorbeeld hier, al heb ik het idee dat de TS díe optie zelf nog niet door had

Ik heb het idee dat TS uberhaupt niet doorheeft hoe onduidelijk (en slecht) zijn codevoorbeeld is met betrekking tot zijn zijn eigen vraagstuk

[GlowMouse]

De or die(mysql_error()) is natuurlijk ook een goede manier om mensen die kwetsbaarheden zoeken te helpen.

Volgende:

PHP:

$id = $_GET['id'];
if($id != (int)$id) {
    die('Henk gaat jou opeten!');
}
$res = mysql_query('SELECT content FROM table WHERE public=1 AND id=' . $id);
if(mysql_num_rows($res) == 0) {
    die('Henk gaat jou opeten!');
}
$content = mysql_result($res, 0, 'content');
echo htmlspecialchars($content);

Bosmonster schreef op vrijdag 16 mei 2008 @ 17:31:
Dit levert overigens nog steeds een dikke foutmelding op (en/of onvoorspelbaar gedrag van de rest van de code) als ik een niet bestaande ID meegeef.

Niet echt een kwetsbaarheid, maar goed, aangepast.

[ Voor 33% gewijzigd door GlowMouse op 16-05-2008 17:33 ]

[Verwijderd]

Al snel veel reacties!

Excuseer mij voor de onlogische code, maar exploits worden toch op bad code gebaseerd? Ik denk niet dat er iemand expres exploits in zijn script gaat steken. Dit is daar zo'n voorbeeld van.
Maar ik ben misschien inderdaad het registratiegedeelte erbij vergeten zetten.. Maar ik heb het juiste antwoord toch al gezien & er werd al een nieuwe geplaatst.

Nogmaal mijn excuses voor onduidelijkheden.


Cheers,
~ GuShe

[Bosmonster]

GlowMouse schreef op vrijdag 16 mei 2008 @ 17:21:
De or die(mysql_error()) is natuurlijk ook een goede manier om mensen die kwetsbaarheden zoeken te helpen.

Volgende:

PHP:

$id = $_GET['id'];
if($id != (int)$id) {
    die('Henk gaat jou opeten!');
}
$res = mysql_query('SELECT content FROM table WHERE public=1 AND id=' . $id);
$content = mysql_result($res, 0, 'content');
echo htmlspecialchars($content);

Dit levert overigens nog steeds een dikke foutmelding op (en/of onvoorspelbaar gedrag van de rest van de code) als ik een niet bestaande ID meegeef.

[.oisyn]

PHP:

$id != (int)$id

Dit geeft iig al false als $id gelijk is aan "aap" oid. (oftewel, "aap" == (int)"aap" - gaat natuurlijk op voor elke string dat 0 oplevert bij de conversie naar int). En maar goed ook, want je id moet immers altijd einigen met een apostroph, anders is de query in regel 5 fout

[ Voor 17% gewijzigd door .oisyn op 16-05-2008 17:37 ]

[GlowMouse]

.oisyn schreef op vrijdag 16 mei 2008 @ 17:34:

PHP:

$id != (int)$id

Dit geeft iig al false als $id gelijk is aan "aap" oid. (oftewel, "aap" == (int)"aap"). En maar goed ook, want je id moet immers altijd einigen met een apostroph, anders is de query in regel 5 fout

Inderdaad: wanneer je een string en een integer vergelijkt, cast php de string naar een integer, zodat dit altijd waar is (ook bij een string als "123 aap"). Next!
Waarom de query fout zou gaan zie ik niet.

[ Voor 7% gewijzigd door GlowMouse op 16-05-2008 17:38 ]

[.oisyn]

Je mist een eind-apostroph na $id Oh nvm, dat is de string-einde-apostroph van PHP zelf

Maar je codevoorbeeld heeft op zich weinig met kwetsbaarheden te maken. Je check is niet goed, maar mysql ondersteunt niet meerdere queries in een enkele mysql_query(), en je select ook alleen maar 'content' uit 'table'. Je kan er nog mooie outer joins aan koppelen om je db om zeep te helpen wellicht, maar andere data dan 'content' uit 'table' kun je er volgens mij niet mee lezen.

[ Voor 7% gewijzigd door .oisyn op 16-05-2008 17:42 ]

[GlowMouse]

.oisyn schreef op vrijdag 16 mei 2008 @ 17:41:
Maar je codevoorbeeld heeft op zich weinig met kwetsbaarheden te maken. Je check is niet goed, maar mysql ondersteunt niet meerdere queries in een enkele mysql_query(), en je select ook alleen maar 'content' uit 'table'. Je kan er nog mooie outer joins aan koppelen om je db om zeep te helpen wellicht, maar andere data dan 'content' uit 'table' kun je er volgens mij niet mee lezen.

De public=1 kun je omzeilen via $id = '0 OR id=[wat je wilt]', althans hier met MySQL 5.0.41. Daarnaast is met een UNION nog een en ander mogelijk.

[ Voor 4% gewijzigd door GlowMouse op 16-05-2008 17:47 ]

[Verwijderd]

Inderdaad: wanneer je een string en een integer vergelijkt, cast php de string naar een integer, zodat dit altijd waar is (ook bij een string als "123 aap"). Next!
Waarom de query fout zou gaan zie ik niet.

In de query staat:

code:

id=' . $id");

als je bijvoorbeeld $id = 567 hebt word het:

code:

id=' . 567);

op het einde mankeert de apostrophe, id word niet 'afgesloten'.

Trouwens, zou bijvoorbeeld:

code:

id='$id');

door php niet goedgekeurd worden? $id werd immers niet Escaped.


Cheers,
~ GuShe

[.oisyn]

Verwijderd schreef op vrijdag 16 mei 2008 @ 17:46:
[...]


In de query staat:

code:

id=' . $id");

Nee, dat staat er dus niet. Ik las het verkeerd

[Verwijderd]

.oisyn schreef op vrijdag 16 mei 2008 @ 17:48:
[...]

Nee, dat staat er dus niet. Ik las het verkeerd

Dan houd ik me er wel buiten voor de rest; ik blijk te veel onzin te verkopen.

[.oisyn]

Ok, ik dan ook maar een

C:

#include <stdio.h>

void eindFunctie()
{
    puts("doei!\n");
}

void eindFunctieVoorOisyn()
{
    puts("dag, oppergod!\n");
}

int main()
{
    char naam[256];
    void (*eindFunctiePtr)() = eindFunctie;

    puts("Geef je naam: ");
    gets(naam);
    if (!strcmp(naam, ".oisyn"))
        eindFunctiePtr = eindFunctieVoorOisyn;

    printf("Hallo, %s!\n", naam);

    eindFunctiePtr();
}

[Sebazzz]

Als je een naam invoert met meer dan 256 tekens heb je de mogelijkheid om kwaadaardige code uit te voeren (in MS termen).

[elTigro]

zou het zijn dat als je bij het vragen van de naam wat meer info intypt als 256 characters je de eindfunctiepointer over kunt schvrijven (vooral omdat je gets gebruikt) en wellicht 'eigengekozen' code kunt uitvoeren?

Sebazzz schreef op vrijdag 16 mei 2008 @ 18:03:
Als je een naam invoert met meer dan 256 tekens heb je de mogelijkheid om kwaadaardige code uit te voeren (in MS termen).

hee, dat wou ik ook zeggen

[ Voor 37% gewijzigd door elTigro op 16-05-2008 18:05 ]

[.oisyn]

Ja maar hoe, dat is de vraag . En dan heb ik het nog makkelijk gemaakt

@elTigro: idd. Je zou kunnen denken aan een system("format c:"), door na de 256 bytes van de naam 4 bytes te gebruiken om de eindFunctie pointer naar system() te wijzen, daarna nog wat ruimte reserveren voor de referentie naar en de string "format c:" zelf. Of gewoon simpelweg wat assembly code waar je 'eindFunctiePtr' weer naar laat wijzen.

[ Voor 75% gewijzigd door .oisyn op 16-05-2008 18:12 ]

[elTigro]

ja, ik ben daar niet zo'n held in, maar je moet dus weten waar bepaalde code zich in het geheugen bevindt, en het adres daarvan dus na die 256 chars plaatsen. wellicht, moet je even zelf een programmatje maken, dat zo'n stuk code alvast definieert, daar het adres even van onthouden, en dit adres vervolgens gebruiken als char[256 +] zeg maar.

juist, spuit '0xb' dus

/edit2:
ik was naar een recent voorbeeld van hier gebruikte code met 'gaatjes' aan het zoeken, maar ik kan het niet meer vinden.
Als iemand al een goed voorbeeld weet, voel je vrij het alvast te posten, ik moet namelijk nodig ervandoor

[ Voor 28% gewijzigd door elTigro op 16-05-2008 18:22 ]

[xos]

Dan doe ik ook een poging, onderstaande functie wordt geinclude & aangeroepen op iedere php pagina om te controleren of de gebruiker de pagina mag bekijken onder de huidige credentials.

PHP:

function authenticate( $user, $passwd )
{
    // pagina's waar geen authenticatie nodig is
    if( strpos( $PHP_SELF, "index.php" ) !== 0 ) return;

    // pagina's waar wel authenticatie vereist voor is, 
    // dus controleer gebruikersnaam/wachtwoord combinatie
    if( !checkCredentials( $user, $passwd ) )
    {
        header( "Location: /login.php" );
    }
}

Bovenstaande code bevat 2 fouten.

[SchizoDuckie]

xos schreef op vrijdag 16 mei 2008 @ 20:10:
Dan doe ik ook een poging, onderstaande functie wordt geinclude & aangeroepen op iedere php pagina om te controleren of de gebruiker de pagina mag bekijken onder de huidige credentials.

PHP:

function authenticate( $user, $passwd )
{
    // pagina's waar geen authenticatie nodig is
    if( strpos( $PHP_SELF, "index.php" ) !== 0 ) return;

    // pagina's waar wel authenticatie vereist voor is, 
    // dus controleer gebruikersnaam/wachtwoord combinatie
    if( !checkCredentials( $user, $passwd ) )
    {
        header( "Location: /login.php" );
    }
}

Bovenstaande code bevat 2 fouten.

Sorry, maar ze zijn misschien wel een beetje simpel... $PHP_SELF bewijst al gelijk dat je register_globals verkeerd hebt ingesteld, en verder hoor je een check op !== false te doen bij strpos.

Dan hebben we het natuurlijk verder even helemaal niet over het feit dat zo'n security check als deze überhaupt best omslachtig is maar verder zie ik nou niet zo 123 wat hier dan erg exploitable aan zou moeten zijn zoals gedefinieerd in de TS

[ Voor 5% gewijzigd door SchizoDuckie op 16-05-2008 20:20 ]

[curry684]

Hoezo 2?
• PHP_SELF is een member van de $_SERVER superglobal, geen variabele.
• PHP_SELF bevat het volledige pad naar het huidige script relatief ten opzichte van de document root. Dit zal dus voor http://www.bier.com/index.php in principe /index.php zijn. Omdat je niet op de slash checkt failt je self-test in regel 4.
• PHP_SELF schijnt nogal wat eigenaardigheden te hebben onder verschillende omgevingen waardoor je beter af bent met intelligentere logica.
• Na een header 'Location' dien je processing van het script te stoppen om de redirect in te zetten.
• Een location header dient volgens de HTTP-specificaties een volledig URL inclusief protocol en FQDN te bevatten.

SchizoDuckie schreef op vrijdag 16 mei 2008 @ 20:18:
[...]

maar verder zie ik nou niet zo 123 wat hier dan erg exploitable aan zou moeten zijn zoals gedefinieerd in de TS

Door het vergeten van de 'die' na het zetten van de location header wordt de rest van het script volledig uitgevoerd alvorens de redirect wordt ingezet. Hierdoor kun je dus postbacks uitvoeren die je niet zou mogen uitvoeren, of data zien die je niet zou mogen zien (moet je wel een custom browser hebben die 3xx responses negeert of telnetten).

[ Voor 30% gewijzigd door curry684 op 16-05-2008 20:23 ]

[dusty]

En natuurlijk kan je gewoon "http://host.tld/mysecuredpage.php?test=index.php" doen waardoor de security een true terug geeft aangezien het index.php heeft en dus niet specifiek controleert of deze persoon wel toegang heeft tot de betreffende "beveiligde" pagina.

[stereohead]

Lol, volgens mij ga ik echt veel leren van dit topic

Edit:
@dusty:
Nee toch, want PHP_SELF bevat toch alleen de scriptnaam, en niet de GET vars?

[ Voor 46% gewijzigd door stereohead op 16-05-2008 20:34 ]

[Verwijderd]

Ik sluit me aan bij de bovenstaande, leuk spelletje, en door uitleg ook erg leerzaam

[xos]

Idd regel 4 is niet correct en na regel 10 hoort het verzoek gestopt te worden. In opera kan je bv aangeven "automatisch omleiden" te willen negeren. Dus curry684 & dusty hebben het beide het juiste eind, doch moet het http://server/private.php/login.php worden

[ Voor 39% gewijzigd door xos op 16-05-2008 20:45 ]

[dusty]

Klopt, mijn fout (tijdje geleden geweest dat ik echt met PHP bezig ben geweest.) Als ik het goed heb is er wel ooit een bug geweest in PHP_SELF waardoor je meer dan alleen de scriptnaam in die string kon krijgen..

edit:

Ah, afhangend van je webserver kan je http://domein.tld/secured.php/index.php?blaat doen.. de secured.php wordt uitgevoerd, en $PHP_SELF kan dan wel eens de "/secured.php/index.php" weergeven..

[ Voor 33% gewijzigd door dusty op 16-05-2008 20:49 ]

[EdwinG]

Afgezien van de hierboven genoemde punten, als deze check overal wordt uitgevoerd, heb je een oneindige loop zodra je login.php benaderd. (of een willekeurige pagina zonder index.php in de naam, terwijl je niet ingelogd bent)

[GlowMouse]

Het is lastig om zonder langere stukjes code toch kwetsbaarheden in te bouwen die niet gelijk zichtbaar zijn. Hier een scriptje om te voorkomen dat eenmaal sniffen volstaat om iemands wachtwoord af te pakken: _{js is ongetest}

PHP:

define('PASSWORD', 'henkpwnz');
session_start();

if(isset($_GET['login'])) {
    if(isset($_POST['password']) && $_SESSION['validlogin'] == $_POST['password']) {
        // glory
    } else {
        die('Henk gaat jou opeten!');
    }
} else {
    $hash = md5( microtime() . rand(1,10000) );
    $_SESSION['validlogin'] = md5($hash . PASSWORD);

    echo '<script language="JavaScript" src="js/md5.js">
        <form method="post" action="' . $_SERVER['PHP_SELF'] . '?login" onSubmit="javascript:password.value=md5(' . $hash . ' + password.value )">
        <input type="password" name="password">
        <input type="submit" value="ogog!">
        </form>';
}

[EdwinG]

GlowMouse schreef op vrijdag 16 mei 2008 @ 21:15:
Het is lastig om zonder langere stukjes code toch kwetsbaarheden in te bouwen die niet gelijk zichtbaar zijn. Hier een scriptje om te voorkomen dat eenmaal sniffen volstaat om iemands wachtwoord af te pakken: _{js is ongetest}

PHP:

define('PASSWORD', 'henkpwnz');
session_start();

if(isset($_GET['login'])) {
    if(isset($_POST['password']) && $_SESSION['validlogin'] == $_POST['password']) {
        // glory
    } else {
        die('Henk gaat jou opeten!');
    }
} else {
    $hash = md5( microtime() . rand(1,10000) );
    $_SESSION['validlogin'] = md5($hash . PASSWORD);

    echo '<script language="JavaScript" src="js/md5.js">
        <form method="post" action="' . $_SERVER['PHP_SELF'] . '?login" onSubmit="javascript:password.value=md5(' . $hash . ' + password.value )">
        <input type="password" name="password">
        <input type="submit" value="ogog!">
        </form>';
}

Direct een post sturen (vanaf lokale pagina bijvoorbeeld) voordat een sessie gestart wordt. Op die manier is $_SESSION['validlogin'] undefined. Dit wordt bij de vergelijking gezien als een lege string.

De $_POST['password'] een lege string laten zijn is ook niet zo moeilijk.

// Ik ben bezig met iets in elkaar te zetten, mocht iemand anders een leuk stukje hebben, plaats die dan gerust.

[ Voor 4% gewijzigd door EdwinG op 16-05-2008 21:33 ]

[GlowMouse]

EdwinG schreef op vrijdag 16 mei 2008 @ 21:30:
[...]

Direct een post sturen (vanaf lokale pagina bijvoorbeeld) voordat een sessie gestart wordt. Op die manier is $_SESSION['validlogin'] undefined. Dit wordt bij de vergelijking gezien als een lege string.

De $_POST['password'] een lege string laten zijn is ook niet zo moeilijk.

Correct! Volgende?

[Cartman!]

Je zou password moeten overschrijven ipv. de hash erbij vermelden. Overigens is language="JavaScript" niet de manier maar zou je type="text/javascript" moeten gebruiken.

edit: gewoon niet goed zitten kijken, dat was em niet en tevens is het goede antwoord al gepost =)

Nieuwe:

PHP:

$strSql = '
    INSERT INTO
        video
    SET
        filename = "' . $strFilename . '",
        hash = "' . sha1(time()) . '"
    ';

Lijkt me niet al te moeilijk wat hier verkeerd kan gaan maar toch zie ik collega's t weleens doen

[ Voor 10% gewijzigd door Cartman! op 16-05-2008 21:34 ]

[SchizoDuckie]

Oke, na nog wat overpeinzingen

• Bij een insert doe je naar mijn weten geen set maar (.<fields>.) VALUES ( .<values>. ) (Of is dit weer een of ander vaag SQL dialect? )
• Is HASH() niet toevallig een interne mysql functie? ( backticks )
• Never trust user input, ik hoop dan ook dat een addslashes of escape oid al over je vars zit.

en $strThis, $strThat is ook best loos in een loose typed taal

[ Voor 141% gewijzigd door SchizoDuckie op 16-05-2008 23:16 ]

[EdwinG]

Eigenlijk wordt bij de bovengenoemde code helemaal niets uitgevoerd richting DB, addslashes heb je dus ook niet nodig. (sowieso zijn er betere alternatieven voor addslashes)
Volgende puzzel dan maar

PHP:

$gevraagd = isset($_GET['bestand']) ? $_GET['bestand'] : '';
if ( $gevraagd != ''
  && !preg_match('#\.\.#', $gevraagd)
  && !preg_match('#^/#', $gevraagd)
  && !preg_match('#\.php#', $gevraagd) ) {
    foutloze_functie_voor_de_headers($gevraagd);
    readfile($gevraagd);
}

Dit script is geplaatst in een directory waarvan de bestanden (en die in subdirectory's) door iedereen bekeken mogen worden.

[ Voor 35% gewijzigd door EdwinG op 16-05-2008 22:13 ]

[Cartman!]

SchizoDuckie : nee, dat is niet alles waar ik op doelde, denk verder... en $str in een loosetyped taal is juist erg handig. zo weet je toch nog wat het is achteraf.

EdwinG: nee slimmert, dat er geen db call wordt gedaan boeit in het voorbeeld niet, zie je me wel connecten dan?

[ Voor 32% gewijzigd door Cartman! op 16-05-2008 22:23 ]

[Gerco]

EdwinG schreef op vrijdag 16 mei 2008 @ 22:03:

PHP:

...
    readfile($gevraagd);
...

Dit script is geplaatst in een directory waarvan de bestanden (en die in subdirectory's) door iedereen bekeken mogen worden.

readfile() accepteert URLs wanneer url_fopen_wrappers aanstaat. Op die manier kun je file:///etc/passwd (bijvoorbeeld) opgeven en op die manier elke willekeurige file op het systeem uitlezen. Dit pas vanaf PHP5, in PHP4 kon dit nog niet.

[ Voor 4% gewijzigd door Gerco op 16-05-2008 22:26 ]

[EdwinG]

Cartman! schreef op vrijdag 16 mei 2008 @ 22:20:
SchizoDuckie : nee, dat is niet alles waar ik op doelde, denk verder... en $str in een loosetyped taal is juist erg handig. zo weet je toch nog wat het is achteraf.

EdwinG: nee slimmert, dat er geen db call wordt gedaan boeit in het voorbeeld niet, zie je me wel connecten dan?

Ok, even serieus dan:
1: Persoonlijk zou ik de " en ' omwisselen
2: Ik hoop dat de hash kolom geen unieke waarde hoeft te hebben, dan zou twee gelijktijdige inserts problemen op gaan leveren
3: Inderdaad escapen van de filename

[EdwinG]

Gerco schreef op vrijdag 16 mei 2008 @ 22:25:
[...]

readfile() accepteert URLs wanneer url_fopen_wrappers aanstaat. Op die manier kun je file:///etc/passwd (bijvoorbeeld) opgeven en op die manier elke willekeurige file op het systeem uitlezen. Dit pas vanaf PHP5, in PHP4 kon dit nog niet.

Dat was nog niet eens waar ik aan gedacht had.
Ik controleer namelijk wel op een / aan het begin, maar een begin in de vorm van c:\ komt wel door de controles heen. (werkt dus alleen i.c.m. Windows)

[Megamind]

Gerco schreef op vrijdag 16 mei 2008 @ 22:25:
[...]

readfile() accepteert URLs wanneer url_fopen_wrappers aanstaat. Op die manier kun je file:///etc/passwd (bijvoorbeeld) opgeven en op die manier elke willekeurige file op het systeem uitlezen. Dit pas vanaf PHP5, in PHP4 kon dit nog niet.

Maar

PHP:

preg_match('#^/#', $gevraagd)

Verbied het opvragen van files met een / erin, right?
_{mijn regex is niet zo super}

[Gerco]

@Megamind: Nee, alleen een / aan het begin van de string (door de ^ )

@EdwinG: En vanwege de controle op ^/ was ik er vanuit gegaan dat dit alleen op *nix zou draaien

[ Voor 42% gewijzigd door Gerco op 16-05-2008 22:37 ]

[Patriot]

Cartman! schreef op vrijdag 16 mei 2008 @ 22:20:
SchizoDuckie : nee, dat is niet alles waar ik op doelde, denk verder... en $str in een loosetyped taal is juist erg handig. zo weet je toch nog wat het is achteraf.

EdwinG: nee slimmert, dat er geen db call wordt gedaan boeit in het voorbeeld niet, zie je me wel connecten dan?

Je levert zelf anders ook wel erg weinig context hoor. Je wil gewoon zeggen dat men dingen moet escapen, maar sommige dingen hoef je echt niet te escapen. Als je als developer de gebruikte variabele declareert is het niet nodig (mits er geen tekens in staan die de boel opfucken), en zéker geen exploit.

EDIT:

En wat SchizoDuckie bedoelt is dat je impliceert dat het één bepaald type is, terwijl het zomaar een ander type zou kunnen zijn (maar daar ben je zelf bij I guess).

[ Voor 11% gewijzigd door Patriot op 16-05-2008 22:40 ]

[EdwinG]

Gerco schreef op vrijdag 16 mei 2008 @ 22:34:
@EdwinG: En vanwege de controle op ^/ was ik er vanuit gegaan dat dit alleen op *nix zou draaien

Die fout heb ik zelf ook gemaakt, en vervolgens het scriptje direct kopieren naar mijn laptop om even snel wat bestanden te delen.

[SchizoDuckie]

@EdwinG's post:

De eerste opmerking is: waarom nederlands gebruiken in je code? *ieuw* (persoonlijke afkeer)

Verder zou ik het zo doen:

[php]
// ff uit het blote hoofd, niet gechecked op syntax enzo

$baseDir = "./mysecretdir/";
$filename = (array_key_exists('bestand', $_GET)) ? basename("{$_GET['bestand']}.php") : false;
// array_key_exists gooit geen foutmelding bij hogere errorlevels. basename regelt ook gelijk slashes.
if ( $filename && file_exists("{$baseDir}{$filename}") && is_readable("{$baseDir}{$filename}") )
{
get_headers_for_file("{$baseDir}{$filename}");
readfile("{$baseDir}{$filename}");
}
?>
[/php]

edit:
OK M'n alcohol begint in te kicken ofzo. Ik heb verkeerd het doel van je originele code gelezen geloof ik.
De WTF in m'n hoofd is nu alleen maar groter. waarom überhaupt mensen een volledige filename op laten geven via GET

[ Voor 28% gewijzigd door SchizoDuckie op 16-05-2008 23:29 ]

[ChessSpider]

Ok, deze is denk ik wat moeilijker.
Deze werkt alleen in IE.. Opzich ook in FF, maar dan doe je het meestal bewust.

PHP:

<?php
error_reporting(E_ALL);
?><html>
<head>
<title>example</title>
</head>
<body>

<?php 

$string = isset($_GET['page']) ? $_GET['page'] : "";

echo htmlentities($string);
echo "<br /><br /><br />";

?>


<form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="get">
<input type="text" name="page" value="" />
<input type='submit' name='submit' value='submit' />
</form>
</body>
</html>

Het is niet de PHP_SELF, ookal is dat volgens mij ook een security risico


ps. Het is mij niet gelukt in IE8, maar wel op IE7..

[ Voor 4% gewijzigd door ChessSpider op 16-05-2008 23:50 ]

[Patriot]

Ieuw. Zet je variabelen eens buiten quotes..

EDIT: Deze was niet @ hierboven.

[ Voor 32% gewijzigd door Patriot op 16-05-2008 23:51 ]

[frickY]

@ChessSpider
Je wilt beweren dat die code een XSS-lek bevat

Je zou hoogstens page als array kunnen inkoppen, waardoor htmlentities een foutmelding geeft. Maar dat is geen exploit.

[ Voor 49% gewijzigd door frickY op 16-05-2008 23:58 ]

[Patriot]

frickY schreef op vrijdag 16 mei 2008 @ 23:56:
@ChessSpider
Je wilt beweren dat die code een XSS-lek bevat

Je zou hoogstens page als array kunnen inkoppen, waardoor htmlentities een foutmelding geeft. Maar dat is geen exploit.

Een array via GET? Kan dat?

[frickY]

Geen probleem;

probeermaar.php

PHP:

<?php
print_r($_GET);
?>

Resultaa van probeermaar.php?var%5B%5D=waarde1&var%5B%5D=waarde2

code:

Array
(
    [var] => Array
        (
            [0] => waarde1
            [1] => waarde2
        )
)

5B en 5D zijn de hexwaarden voor square brackets

[ Voor 82% gewijzigd door frickY op 17-05-2008 00:08 ]

[ChessSpider]

frickY schreef op vrijdag 16 mei 2008 @ 23:56:
@ChessSpider
Je wilt beweren dat die code een XSS-lek bevat

Je zou hoogstens page als array kunnen inkoppen, waardoor htmlentities een foutmelding geeft. Maar dat is geen exploit.

Yup, XSS lek.
Tip; character encoding.

Deze lek zal niet voor iedereen werken. Zo heb je bijvoorbeeld een IE browser nodig waarbij (Character-)Encoding op Auto-Select staat. Bij mij lukte het niet om IE8 op auto-select te zetten, maar wel op IE7. Als je wilt kan je hier testen;

http://chessspider.no-ip.info/blaat.php?

[ Voor 29% gewijzigd door ChessSpider op 17-05-2008 00:06 ]

[Patriot]

ChessSpider schreef op zaterdag 17 mei 2008 @ 00:04:
[...]


Yup, XSS lek.
Tip; character encoding.

Deze lek zal niet voor iedereen werken. Zo heb je bijvoorbeeld een IE browser nodig waarbij (Character-)Encoding op Auto-Select staat. Bij mij lukte het niet om IE8 op auto-select te zetten, maar wel op IE7. Als je wilt kan je hier testen;

http://chessspider.no-ip.info/blaat.php?

WTF! Dat is een goeie, dát wist ik niet.

EDIT: Wacht.. waarom werkt het op mijn localhost niet..\

EDIT: -- Nog even niet --

[ Voor 44% gewijzigd door Patriot op 17-05-2008 00:12 ]

[SchizoDuckie]

Patriot schreef op zaterdag 17 mei 2008 @ 00:00:
[...]


Een array via GET? Kan dat?

Sure:

code:

blaat.php?array[meuk]=kwaak&array[]=blaat&array[]=woei

geeft:

PHP:

print_r($_GET['array']);
/* 
Array ( 'meuk'=> 'kwaak',
0 => 'blaat'
1 => 'woei') */

[/ te laat ]

[ChessSpider]

Patriot schreef op zaterdag 17 mei 2008 @ 00:07:
[...]


WTF! Dat is een goeie, dát wist ik niet.

EDIT: Wacht.. waarom werkt het op mijn localhost niet..

Wacht nog even met zeggen

[SchizoDuckie]

ChessSpider schreef op zaterdag 17 mei 2008 @ 00:11:
[...]


Wacht nog even met zeggen

laat me raden... local intranet zone?

[ChessSpider]

SchizoDuckie schreef op zaterdag 17 mei 2008 @ 00:17:
[...]

laat me raden... local intranet zone?

Neej, ga even zoeken gast

[Patriot]

Sorry, maar ik ga het nu toch echt (sortof) verklappen (dat is uiteindelijk ook gewoon de bedoeling ).
Ik snap dat het komt door de character encoding. Wat ik echter niet begrijp is waarom het niet in Fx werkt. Het komt er toch op neer dat als je de page-variabele als UTF-8 verstuurd, htmlentities() deze niet escaped?

[ChessSpider]

Patriot schreef op zaterdag 17 mei 2008 @ 00:29:
Sorry, maar ik ga het nu toch echt (sortof) verklappen (dat is uiteindelijk ook gewoon de bedoeling ).
Ik snap dat het komt door de character encoding. Wat ik echter niet begrijp is waarom het niet in Fx werkt. Het komt er toch op neer dat als je de page-variabele als UTF-8 verstuurd, htmlentities() deze niet escaped?

Als IE7 in de eerste 4096 characters een UTF-7 encoded string vind, terwijl er geen character encoding word meegestuurd, maakt hij er automatisch UTF-7 van.
De htmlentities() weet niet dat de character encoding als UTF-7 word gezien, dus die ziet letterlijk dit voorbij komen:

code:

+ADw-script+AD4-alert(document.location)+ADw-/script+AD4-

Daar gaat hij niks escapen, en word het zo naar de browser gestuurd. IE herkent het als UTF-7, en veranderd de character encoding accordingly.

Dit kan natuurlijk alleen als:
Er geen character encoding is meegegeven
Character encoding op auto-select staat

Probeer bovenstaande tekst te kopieren en te plakken in de voorbeeld. Als het goed is krijg je dan een alertbox te zien met de URL. Je kan het ook forceren; bijvoorbeeld door in FireFox naar de URL te gaan, en dan handmatig de character encoding op UTF-7 te zetten.


Fx verandert dus niet zijn encoding opeens naar UTF-7.

Hier nog een voorbeeld en wat uitleg, waar ik mijn voorbeeld ook grotendeels van heb gestolen:
http://shiflett.org/blog/2005/dec/google-xss-example

[ Voor 7% gewijzigd door ChessSpider op 17-05-2008 00:38 ]

[Patriot]

ChessSpider schreef op zaterdag 17 mei 2008 @ 00:35:
[...]


Als IE7 in de eerste 4096 characters een UTF-7 encoded string vind, terwijl er geen character encoding word meegestuurd, maakt hij er automatisch UTF-7 van.
De htmlentities() weet niet dat de character encoding als UTF-7 word gezien, dus die ziet letterlijk dit voorbij komen:

code:

+ADw-script+AD4-alert(document.location)+ADw-/script+AD4-

Daar gaat hij niks escapen, en word het zo naar de browser gestuurd. IE herkent het als UTF-7, en veranderd de character encoding accordingly.

Dit kan natuurlijk alleen als:
Er geen character encoding is meegegeven
Character encoding op auto-select staat

Probeer bovenstaande tekst te kopieren en te plakken in de voorbeeld. Als het goed is krijg je dan een alertbox te zien met de URL. Je kan het ook forceren; bijvoorbeeld door in FireFox naar de URL te gaan, en dan handmatig de character encoding op UTF-7 te zetten.

Hmm, nou, het is een aparte. Anyway, ik zal hem niet vaak tegenkomen omdat ik sowieso een charset meestuur.

[user109731]

Er staat er geeneen meer open, dus maar een nieuwe. Een sprintf-like functie om query parameters te escapen:

PHP:

function format_query() {
    $args = func_get_args();
    if(count($args) == 0)
        die('error');

    $query = array_shift($args);
    $expected_args = substr_count($query, '?');

    if(count($args) != $expected_args) {
        printf('expected %d args, but got %d', $expected_args, count($args));
        exit();
    }
    foreach($args as $arg) {
        // replace next ? with next argument
        $arg = my_escape($arg); // my_escape is safe for all input
        $query = preg_replace('#\?#', $arg, $query, 1);
    }
    return $query;
}

Kan zo gebruikt worden:

PHP:

$sql = format_query("SELECT * FROM table WHERE x = '?' AND y = ?", "foo", 5);

Iemand?

[ Voor 10% gewijzigd door user109731 op 17-05-2008 08:52 ]

[H!GHGuY]

doordat die my_escape lokaal op 1 argument werkt weet je niet of de query zelf ' ' bevat.
(zie voorbeeld: x = '?' en y = ?)

Daardoor kun je denk'k alsnog SQL-injection doen.

(ik ken helemaal de ballen van php, dus ik blaat maar wat aan)

Next?

C++:

class CFixedSizeString
{
  public:
     CFixedSizeString(const std::string& str)
     {
        if (strlen(str.c_str()) > 255)
           throw "badly sized string";
        strncpy(buffer, str.data(), str.size());
     }
     bool operator<(const CFixedSizeString& right) { return strcmp(buffer, right.buffer) < 0; }
     virtual void print() { std::cout << buffer; }
     virtual ~CFixedSizeString() { }
  private:
    char buffer[256];
};

int main()
{
  std::set<CFixedSizeString> strings;
  std::string str;
  CFixedSizeString fsstr;
  std::cin >> str;
  while (str != "")
  {
    fsstr = CFixedSizeString(str)
    strings.insert(fsstr);
    std::cin >> str;
  }
  std::cout << "Unieke woorden: " << std::endl;
  for (std::set<CFixedSizeString>::iterator it = strings.begin(), end = strings.end(); it != end; ++it)
  {
    it->print();
    std::cout << std::endl;
  }
  return 0;
}

edit: bugfix

[Sebazzz]

Je returned niet uit main() dus deze zal zowieso niet compilen.

@hieronder: Het is in ieder geval een goede gewoonte om het te doen. Je hoeft in HTML4 sommige tags niet af te sluiten, maar het is ook een goede gewoonte om dat te doen

[ Voor 58% gewijzigd door Sebazzz op 17-05-2008 10:03 ]

[H!GHGuY]

afhankelijk van welke compiler en hoe streng de opties staan, that is.
Normaal wordt 0 gereturned als er niets staat.

Heb het even toegevoegd, maar daar zit het probleem niet.

@hierboven: gelukkig is het niet uit gewoonte maar was ik het gewoon even vergeten

[ Voor 34% gewijzigd door H!GHGuY op 17-05-2008 10:07 ]

[PrisonerOfPain]

H!GHGuY schreef op zaterdag 17 mei 2008 @ 09:54:
(ik ken helemaal de ballen van php, dus ik blaat maar wat aan)

.data() geeft een string die niet null-terminated is terwijl strcmp dat wel verwacht, waar door 'ie over je boundries gaat. Zou verder niet weten hoe je er een exploit voor maakt.

PS. je hebt een virtual void ~CFixedSizeString dus 't compiled sowieso niet.

[H!GHGuY]

PrisonerOfPain schreef op zaterdag 17 mei 2008 @ 10:26:
[...]


.data() geeft een string die niet null-terminated is terwijl strcmp dat wel verwacht, waar door 'ie over je boundries gaat. Zou verder niet weten hoe je er een exploit voor maakt.

PS. je hebt een virtual void ~CFixedSizeString dus 't compiled sowieso niet.

Je hebt een 2de bug gevonden, hoewel dit niet die is die ik zoek. Ik kon er even goed strncmp van maken met n=256.
Je bent echter wel op de goede weg.

Voorbeeldje heb ik niet door een compiler gehaald of getest, er kunnen dus idd foutjes zoals die destructor in staan, ik corrigeer het even.


Na even grondig denken over mijn voorbeeld heb ik een aanpassing moeten maken omdat ik even fout zat...

[ Voor 9% gewijzigd door H!GHGuY op 17-05-2008 11:37 ]

[Cartman!]

Patriot schreef op vrijdag 16 mei 2008 @ 22:36:
[...]


Je levert zelf anders ook wel erg weinig context hoor. Je wil gewoon zeggen dat men dingen moet escapen, maar sommige dingen hoef je echt niet te escapen. Als je als developer de gebruikte variabele declareert is het niet nodig (mits er geen tekens in staan die de boel opfucken), en zéker geen exploit.

EDIT:

En wat SchizoDuckie bedoelt is dat je impliceert dat het één bepaald type is, terwijl het zomaar een ander type zou kunnen zijn (maar daar ben je zelf bij I guess).

Het gaat niet zozeer om het escapen dus want als je die waarde zelf samenstelt en je weet dat daar geen onveilige karakters inzitten is er geen probleem. Valt me tegen dat niemand het ziet eigenlijk. Als je een hash geeft aan een entry in een database dan moet je eerst checken of die hash er al niet is... De kans op collisions is niet heel groot maar zeker aanwezig. 2 entries met dezelfde hashes kan een hoop problemen opleveren (kijk bijv. naar youtube die de hash uit de url gebruikt als id).

Nog een keer de code:

PHP:

$strSql = '
    INSERT INTO
        video
    SET
        filename = "' . $strFilename . '",
        hash = "' . sha1(time()) . '"
    ';

En over het prefixen: bij ons op t werk is het een code standaard. Iedereen houdt zich daar keurig aan en dat is echt handig tijdens het doorkijken van een script.

[ Voor 6% gewijzigd door Cartman! op 17-05-2008 11:41 ]

[Cavorka]

Cartman! schreef op zaterdag 17 mei 2008 @ 11:41:
[...]

PHP:

$strSql = '
    INSERT INTO
        video
    SET
        filename = "' . $strFilename . '",
        hash = "' . sha1(time()) . '"
    ';

Zonder user input (hetzij via _POST, of via _GET) valt er natuurlijk weinig te exploiten... Maar misschien zie ik hier gewoon niet veel in, alleen een query is misschien iets te abstract.

Enige wat ik kan verzinnen is dat dit niet handig is en dat je misschien beter dit kan doen:

code:

sha1(time() .  $strFilename)

[Cartman!]

Het probleem is dat je dan nog steeds duplicates kunt krijgen in je database en dat wil je niet. Dus gewoon een functie maken die een hash genereert, checkt of ie niet voorkomt en als dat zo is recursief zichzelf aanroepen om een nieuwe te maken totdat je een unieke hebt. Je gaat toch ook geen nieuwsberichten hetzelfde id geven?

[EdwinG]

Cartman! schreef op zaterdag 17 mei 2008 @ 12:57:
Het probleem is dat je dan nog steeds duplicates kunt krijgen in je database en dat wil je niet. Dus gewoon een functie maken die een hash genereert, checkt of ie niet voorkomt en als dat zo is recursief zichzelf aanroepen om een nieuwe te maken totdat je een unieke hebt. Je gaat toch ook geen nieuwsberichten hetzelfde id geven?

Het probleem is dat met jouw voorbeeld niet direct duidelijk is waar de gemaakte hash voor dient. En het kan net zo goed zijn dat je daarnaast nog een kolom met auto_increment als primary key gebruikt.

[Cartman!]

Maar dan nog, bij Youtube zal iedere video vast een id hebben maar je krijgt deze gepresenteert adhv. de hash. Als een hash niet uniek hoeft te zijn dan heb je er niks aan in een database imo, tenzij deze als checksum dient.

[Sebazzz]

Als je unieke hashes wilt moet je natuurlijk met microtime of iets dergelijks gaan werken. De kans dat je twee gelijke hashes krijgt...

@hieronder: Wel volgens mijn C++ boek (C++ Grand Cru, Steve nogwat)

[ Voor 20% gewijzigd door Sebazzz op 17-05-2008 14:36 ]

[.oisyn]

Sebazzz schreef op zaterdag 17 mei 2008 @ 09:59:
Je returned niet uit main() dus deze zal zowieso niet compilen.

main() behoeft geen return statement. Maar de return-type is wel altijd int.

@hieronder: Het is in ieder geval een goede gewoonte om het te doen

Waarom? main() is sowieso een special case (main() mag bijvoorbeeld ook niet aangeroepen worden door eigen code). 'return x' is in feite hetzelfde als 'exit(x)'. Waarom is dát dan geen goede gewoonte om neer te zetten? Als je een return-waarde onzinnig vind maak je 'm void, dat mag bij main() helaas niet. En in main() is geen return is hetzelfde als return 0 - iedere fatsoenlijke C++er weet dat

H!GHGuY: de bug is dat CFixedString::buffer nooit wordt afgesloten met een terminating 0. Je had gewoon strcpy() moeten gebruiken ipv strncpy(), wat mogelijk is omdat je de lengte daarvoor toch controleert.

[H!GHGuY]

.oisyn schreef op zaterdag 17 mei 2008 @ 14:05:
[...]

main() behoeft geen return statement. Maar de return-type is wel altijd int.


[...]

Waarom? main() is sowieso een special case (main() mag bijvoorbeeld ook niet aangeroepen worden door eigen code). 'return x' is in feite hetzelfde als 'exit(x)'. Waarom is dát dan geen goede gewoonte om neer te zetten? Als je een return-waarde onzinnig vind maak je 'm void, dat mag bij main() helaas niet. En in main() is geen return is hetzelfde als return 0 - iedere fatsoenlijke C++er weet dat

H!GHGuY: de bug is dat CFixedString::buffer nooit wordt afgesloten met een terminating 0. Je had gewoon strcpy() moeten gebruiken ipv strncpy(), wat mogelijk is omdat je de lengte daarvoor toch controleert.

er zit wel een terminating 0 in hoor kijk nog maar eens goed...

Ik ben echter niet helemaal zeker dat het voorbeeld 100% klopt, maar mits minimale aanpassingen moet het toch zeker lukken...

Verklap'k em ?

[ Voor 7% gewijzigd door H!GHGuY op 17-05-2008 14:39 ]

[H!GHGuY]

Ik kan er grandioos naast zitten, maar de redenering is zo:

je buffer moet je zo construeren:

code:

0:[256karakters zonder whitespace met minstens 1 \0 karakter - kan exploit code bevatten]
256:[vptr wijst naar 260]
260:[vptr0 wijst naar 268 of 0]
264:[vptr1 wijst naar 268 of 0]
268:[exploit code]

inlezen van een std::string kijkt naar EOF of whitespace. NUL-karakters worden afaik gewoon mee ingelezen.
strlen leest tot de eerste NUL-byte, str.data geeft alle data + bijhorende .size() (groter dan de strlen)
dus je kopieert over de vptr pointer. Doordat de initiele CFixedSizeString op de stack staat kun je de offsets daarvan gebruiken om tijdens de laatste inleesopdracht je code te injecteren met gekende offsets.

Door uiteindelijk de print() opdracht op te roepen jump je naar je exploit code.

Ik vermoed echter dat de set pointers naar CFixedSizeString zal moeten bevatten of anders wordt tijdens de copy-constructor actie de vptr van het geinserteerde object juist gezet.

[Patriot]

Cartman! schreef op zaterdag 17 mei 2008 @ 13:35:
Maar dan nog, bij Youtube zal iedere video vast een id hebben maar je krijgt deze gepresenteert adhv. de hash. Als een hash niet uniek hoeft te zijn dan heb je er niks aan in een database imo, tenzij deze als checksum dient.

Dat zal allemaal gerust zo zijn hoor, maar niks van dit alles is uit de code te halen die jij geeft. Je geeft alleen de declaratie van een variabele, dat het een query betreft impliceert dan nog dat je het waarschijnlijk uit zou voeren maar dan houdt het toch echt wel op..

De fout is volgens jou dat er niet gekeken word of die hash al bestaat, maar uit je voorbeeld is onmogelijk te halen dat de hash uniek moet zijn. En dat een hash die niet uniek hoeft te zijn niet nuttig is, is ook niet waar. Je komt ze bijvoorbeeld vaak tegen in gebruikerssystemen. Een gebruiker die zijn wachtwoord is vergeten krijgt dan vaak een hash toegestuurd via de mail, waarna hij op een pagina alleen zijn wachtwoord kan veranderen als de hash klopt.

Je voorbeeld is leuk geprobeerd, maar helaas te abstract en ik kan me ook niet voorstellen dat het echt een exploit is.

[Zoijar]

Cartman! schreef op zaterdag 17 mei 2008 @ 12:57:
Het probleem is dat je dan nog steeds duplicates kunt krijgen in je database en dat wil je niet. Dus gewoon een functie maken die een hash genereert, checkt of ie niet voorkomt en als dat zo is recursief zichzelf aanroepen om een nieuwe te maken totdat je een unieke hebt. Je gaat toch ook geen nieuwsberichten hetzelfde id geven?

Op die manier kan het nog steeds mis gaan. Twee users submitted verschillende files met toevallig dezelfde hash waarde. Eerste process queried de database of er duplicates zijn en vindt die niet. Op dat moment kicked het tweede process in, queried ook de database, en vindt ook geen duplicates. Vervolgens gaan ze allebei lekker insert queries draaien, en voila, een duplicate hash.

maw. je moet dan een atomic query maken die eerst select en dan conditioneel insert. Maar dan moet je nieuwe hashes kunnen genereren en loopen in je query: niet handig. Dus moet je je database locken gedurende de tijd dat je select/insert script draait. Dan kan je je site DoS-en door heel veel files te submitten

concurrency exploits zijn natuurlijk ook leuk. Ik kan me wel iets indenken dat een Os een syncrhonized multiple producer/consumer buffer bijhoudt als event queue, maar dat op de simpelete manier met twee semaphores doet. Ik heb geen zin om een voorbeeld te geven, dat wordt te ingewikkeld. Het idee is in ieder geval dat je door een race-conditie naar hetzelfde slot kan schrijven als een ander process, en zo priviliged events zou kunnen faken ....

[ Voor 32% gewijzigd door Zoijar op 17-05-2008 15:48 ]

[.oisyn]

H!GHGuY schreef op zaterdag 17 mei 2008 @ 14:37:
[...]


er zit wel een terminating 0 in hoor kijk nog maar eens goed...

Nou wijs het maar aan dan, want ik zie het niet. strncpy() schrijft maximaal n characters. n is gelijk aan str.size(), dus strncpy() vult buffer met n characters zonder trailing 0.

H!GHGuY schreef op zaterdag 17 mei 2008 @ 14:54:
Ik kan er grandioos naast zitten

klopt

strlen leest tot de eerste NUL-byte, str.data geeft alle data + bijhorende .size() (groter dan de strlen)
dus je kopieert over de vptr pointer.

Fout, om drie redenen. De vtable pointer staat meestal vooraan in de class, en dus niet achter buffer. Dit is wel zo praktisch omdat typeid() en dynamic_cast ook op void pointers moet kunnen werken. Als de vtable pointer achter buffer staat is het daadwerkelijke type zo goed als niet te achterhalen.

De andere reden is dat ook strncpy() niet altijd n characters kopiëert, maar maximaal n characters. Hij ie een 0 tegenkomt en i<n dan stopt dus gewoon. Je functie kopiëert daarom nooit meer dan 255 tekens door de if die ervoor staat. Als je altijd exact n characters wilt kopiëren moet je memcpy() gebruiken. En sowieso zelf de trailing 0 erachter zetten, want ook strncpy() doet dat niet als er al n characters geschreven zijn.

De derde noemde je zelf al:

Ik vermoed echter dat de set pointers naar CFixedSizeString zal moeten bevatten of anders wordt tijdens de copy-constructor actie de vptr van het geinserteerde object juist gezet.

Klopt als een bus

[ Voor 8% gewijzigd door .oisyn op 17-05-2008 22:11 ]

[Verwijderd]

Ach Zojar, jammer dat je nog niet gehoord hebt van Database Transactions. Dat maakt je hypothetisch voorbeeld onmogelijk.

[Verwijderd]

H!GHGuY schreef op zaterdag 17 mei 2008 @ 09:54:

C++:

class CFixedSizeString
{
  public:
     CFixedSizeString(const std::string& str)
     {
        if (strlen(str.c_str()) > 255)
           throw "badly sized string";
        strncpy(buffer, str.data(), str.size());
     }

Normaal gebruik ik geen std::string, maar is deze aanname wel juist: strlen(str.c_str())==str.size(). Als de string meerdere '\0' characters heeft, kan dat evt mis gaan...

[.oisyn]

En als je even door had gelezen dan was je erachter gekomen dat dat idd de clue was

[Zoijar]

Verwijderd schreef op zaterdag 17 mei 2008 @ 21:35:
Ach Zojar, jammer dat je nog niet gehoord hebt van Database Transactions. Dat maakt je hypothetisch voorbeeld onmogelijk.

Het zal je verbazen waar ik allemaal van gehoord heb Zoals ik al zei, dan moet je een atomic query gebruiken, en dat kan idd middels een transaction. Maar aangezien het hier om kwetsbaarheden ging, merk ik op dat dit niet doen ook vaak een kwestbaarheid kan opleveren. Ik heb genoeg code gezien in het verleden die op deze manier records insert: select max(id) from users, fetch record set $result, insert user met id=$result. (in mijn tijd was MySQL overigens de meest gebruikte database icm web-development, en in de oude versies had je helaas niet de luxe van transaction support)

[ Voor 9% gewijzigd door Zoijar op 18-05-2008 11:52 ]

[H!GHGuY]

.oisyn schreef op zaterdag 17 mei 2008 @ 19:12:
[...]

Nou wijs het maar aan dan, want ik zie het niet. strncpy() schrijft maximaal n characters. n is gelijk aan str.size(), dus strncpy() vult buffer met n characters zonder trailing 0.


[...]

klopt

[...]

Fout, om drie redenen. De vtable pointer staat meestal vooraan in de class, en dus niet achter buffer. Dit is wel zo praktisch omdat typeid() en dynamic_cast ook op void pointers moet kunnen werken. Als de vtable pointer achter buffer staat is het daadwerkelijke type zo goed als niet te achterhalen.

De andere reden is dat ook strncpy() niet altijd n characters kopiëert, maar maximaal n characters. Hij ie een 0 tegenkomt en i<n dan stopt dus gewoon. Je functie kopiëert daarom nooit meer dan 255 tekens door de if die ervoor staat. Als je altijd exact n characters wilt kopiëren moet je memcpy() gebruiken. En sowieso zelf de trailing 0 erachter zetten, want ook strncpy() doet dat niet als er al n characters geschreven zijn.

De derde noemde je zelf al:

[...]

Klopt als een bus

niet schieten is altijd mis

Leer ik zelf ook gelijk wat bij...

[EdwinG]

Volgens mij zijn we nu door de puzzels heen. Wie plaatst de volgende?

[.oisyn]

Tja, na de simpele buffer overruns en triviale XSS voorbeelden hebben we het wel zo'n beetje gehad. Ik kan me nog iets voorstellen met een UBB-achtige parser waarin je URLs of IMGs kunt stoppen waarbij je ook op moet passen dat daarin geen javascript oid is uit te voeren, maar dan wordt het voorbeeld al snel te complex om hier te posten.

[Gomez12]

Ach, klassieker die ik bijna wekelijks voorbij zie komen :

code:

$res = mysql_query('SELECT username FROM users WHERE name="pietje";'); 
$row = mysql_fetch_assoc($result);
$query2= mysql_query('update rechten set (adminright=True) where username="' . $row['username'] . '"');

[ Voor 3% gewijzigd door Gomez12 op 20-05-2008 20:34 ]

[Voutloos]

Dankzij het verschil tussen $res en $result gaat daar niets fout, behalve dan dat je een whopping notice krijgt.

[Cartman!]

Het is sowieso niet handig om in de rechtentabel de naam te gebruiken ipv. het userid, is dat wat je bedoeld? Moeten er trouwens geen quotes om True heen? En de haakjes zijn ook overbodig.

offtopic: het adminright=true is ook erg beperkend voor een applicatie...

[Megamind]

Volgens mij zit er geen fout in? Je kan je registreren als piet" OR 1=1 maar ik denk dat niet eens gaat werken (zowiezo niet als je pietje hard erin bakt).

Cartman! schreef op dinsdag 20 mei 2008 @ 21:10:
Het is sowieso niet handig om in de rechtentabel de naam te gebruiken ipv. het userid, is dat wat je bedoeld? Moeten er trouwens geen quotes om True heen? En de haakjes zijn ook overbodig.

offtopic: het adminright=true is ook erg beperkend voor een applicatie...

Dan wordt het meer een "Debug mijn script" topic...

[ Voor 58% gewijzigd door Megamind op 20-05-2008 21:12 ]

[Onbekend]

Wat gebeurt er als $row leeg is?