Is mijn code veilig?

Door Allard Keij op 04-02-2016 19:21

3.946 views

Hallo allemaal,

Ik maak heel mijn leven al gebruik van PHP maar ik heb er nooit voor geleerd en ben dit ook niet van plan aangezien het voor mij altijd een "bijproduct" is.

Ik ontwikkel namelijk applicaties voor Android en iOS. Ik gebruik dus PHP om gegevens in een database te zetten en te lezen.

Nu maak ik dus gebruik van de lelijke $_GET functie aangezien mijn app op deze manier gegevens kan verwerken en ontvangen.

Ik gebruik dit bijvoorbeeld om een comment system in mijn app te bouwen. Nu wil ik natuurlijk wel dat er geen misbruik van gemaakt kan worden. Daarom wil ik even voor de zekerheid checken of mijn code veilig voor gebruik is.

De code die ik momenteel gebruik:


<?php
header('Access-Control-Allow-Origin: *');

$mysqli = new mysqli("*", "*", "*", "*");

/* check connection */
if (mysqli_connect_errno()) {
    printf("Error");
    exit();
}

$username = $_GET['username'];
$fbid = $_GET['fbid'];

function randomCode($length=8) {
   $characters = "23456789ABCDEFHJKLMNPRTVWXYZ";
   for ($p = 0; $p < $length; $p++) 
   {
       $string .= $characters[mt_rand(0, strlen($characters)-1)];
   }
   return $string;
}
$stmt = $mysqli->prepare("INSERT INTO users (fbid,name,discountcode) VALUES ('$fbid', '$username', '" . randomCode(8) . "')");

$stmt->bind_param("s", $username);
$stmt->bind_param("s", $fbid);

$stmt->execute();

printf ($mysqli->insert_id);

$stmt->close();

$mysqli->close();
?>

De code zelf werkt prima, maar ik wil er zeker van zijn dat dit ook veilig is.

Alvast erg bedankt.

- SanThe -

04-02-2016 21:25

Dit: randomCode(8) geeft geen garantie op een unieke code.

Ben van Velzen

04-02-2016 21:34

Ook nog eens. Dit is gelukkig eenvoudig op te vangen dmv een unique constraint en controle hierop. Als de constraint overtreden wordt nieuwe code genereren en opnieuw proberen.

Allard Keij

04-02-2016 21:40

Ik vond persoonlijk de volgorde ook erg raar. Ik heb eigenlijk niks anders gedaan dan het voorbeeld gevolgd na aanleiding van de link die gepost was. Raar genoeg werkt het wel gewoon.

Ik ga ze toch even omwisselen inderdaad al is het alleen maar voor het zicht.

De random code hoeft ook niet random te zijn. Dit is simpel weg een kortingscode voor in de online winkel die altijd geldig zal zijn. Je zou de code dus ook heel vrolijk online kunnen gooien en iedereen zou er gebruik van kunnen maken. Dat is tevens ook mijn bedoeling ;)! Ik hou van mensen die producten kopen omdat ze "korting" krijgen.

In dit geval dus niet erg. Wil alleen niet dezelfde code voor iedereen gebruiken zodat de gebruikers toch een beetje het unieke gevoel krijgen.

Dennis WhoCares

05-02-2016 08:17

time() of microtime() zijn behoorlijk uniek? :D

Allard Keij

11-02-2016 17:52

Het is al weer even geleden maar ik test zojuist mijn code en zie nog altijd problemen.

Mijn huidige code is:


<?php
$mysqli = new mysqli("*", "*", "*", "*");

/* check connection */
if (mysqli_connect_errno()) {
    printf("Error");
    exit();
}

$factid = $_GET['factid'];
$feedid = $_GET['feedid'];
$userid = $_GET['userid'];
$avatarid = $_GET['avatarid'];
$username = $_GET['username'];
$feedcat = $_GET['feedcat'];
$comment = $_GET['comment'];

$stmt = $mysqli->prepare("INSERT INTO comments (feedid, avatarid, username, userid, postid, comment) VALUES (?,?,?,?,?,?,?)");

$stmt->bind_param("ssssss", $feedid, $avatarid, $username, $userid, $factid, $comment);

$stmt->execute();

printf ($mysqli->insert_id);

$stmt->close();

$mysqli->close();
?>

Zoals je ziet werkt alles met een $_GET. Mijn probleem is dat als ik als comment (dit is door de gebruiker zelf in te vullen) bijvoorbeeld: &username=Appel , intyp veranderd de username naar Appel en dat is natuurlijk niet de bedoeling.

In mijn beleving zou mijn bovenstaande code dit moeten voorkomen maar dat is dus niet het geval.

Wat is de juiste weg om dit wel voor elkaar te krijgen? Dus zelfs als ik als comment: &username=Appel intyp zou dat niet meegenomen moeten worden in de code.

Alvast bedankt!

Ben van Velzen

11-02-2016 18:03

Gewoon POST gebruiken ipv GET?

- SanThe -

11-02-2016 18:06

6 stuks: (feedid, avatarid, username, userid, postid, comment)
VALUES
7 stuks: (?,?,?,?,?,?,?)

Allard Keij

11-02-2016 18:27 gewijzigd op 11-02-2016 20:13

My bad, er zitten ook 7 stuks in het eerste gedeelte maar die heb ik er even uitgehaald aangezien dit de naam is van mijn project en die wil ik hier niet terug vinden in Google ;)

Dit is dus ook niet het probleem waarom het niet werkt.

Post is geen optie aangezien het niet gepost kan worden ( deze request komt vanuit een app ).

Dennis WhoCares

11-02-2016 20:17 gewijzigd op 11-02-2016 20:25

Ben van Velzen op 11/02/2016 18:03:27

Gewoon POST gebruiken ipv GET?

Ozzie heeft mij van de week nog iets uitgelegt over wanneer POST of GET te gebruiken :)
GET is om waardes op te halen, POST om gegevens op te slaan
De benoeming van de 'request method' geeft zijn betekenis eigenlijk al weer.

Allard Keij op 11/02/2016 18:27:11

Post is geen optie aangezien het niet gepost kan worden ( deze request komt vanuit een app ).

Weet je zeker dat dit niet kan ? Zou wel een down-side zijn om uberhaubt te gebruiken voor bijv. login's

Allard Keij

11-02-2016 20:25 gewijzigd op 11-02-2016 20:26

Ondanks dat heb ik helaas geen andere mogelijkheid dan mijn waarden door te geven via de link :(.

Je zou toch zeggen dat je ergens kan checken hoeveel variables er binnen komen?

Zoals in dit script hierboven mogen er maar 7 binnen komen. Mocht dit te checken zijn zou het opgelost zijn.

Mits er natuurlijk geen andere security issues ontstaan aangezien het feit blijft dat mensen kunnen typen wat ze willen als &comment=DIT HEB IK NIET IN BEHEER.

Reageren

Inloggen om te reageren