Is mijn code veilig?

Overzicht Reageren

Sponsored by: Vacatures door Monsterboard

Full Stack Developer Industriële Automatiseri

Raster wordt continu betrokken bij complexe en baanbrekende projecten in industriële automatisering. Ons team is gespecialiseerd in productie automatisering en proces automatisering projecten waarin procesveiligheid een belangrijk aspect is. Ons compact en groeiend OX (Operational Excellence) team ontwikkelt, configureert en levert support op zelfontwikkelde applicaties waarmee onze klanten hun productieprocessen slimmer, sneller, goedkoper en veiliger maken. Operational Excellence wordt zichtbaar gemaakt in resultaten. Bij bedrijven met dezelfde strategie, zal een operationeel excellent bedrijf een lager operationeel risico, lagere operationele kosten hebben en relatief meer winst maken. Het Raster OX team laat haar klanten de resultaten behalen door hun eigen procesdata

Bekijk vacature »

Pagina: 1 2 volgende »

Allard Keij

Allard Keij

04/02/2016 19:21:59
Quote Anchor link
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:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
<?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.
 
PHP hulp

PHP hulp

20/10/2021 02:05:07
 
Randy vsf

Randy vsf

04/02/2016 19:29:18
Quote Anchor link
Ik zie nergens een check of de aanroep wel vanuit je app komt, dat zou ik er wel nog bijzetten.
 
Allard Keij

Allard Keij

04/02/2016 19:32:57
Quote Anchor link
Dat zou ik heel graag doen maar ik heb helaas die mogelijkheid niet. Ik maak gebruik van een soort drag & drop software en het word ge-export als HTML5 project. De software zelf heeft helaas geen functie om bijvoorbeeld een unieke check te doen.

Vanuit mijn software worden de php scripts aangeroepen door een AJAX request.

Verder is de code dus wel veilig voor SQL injection e.d.?

Bedankt voor je antwoord!
Gewijzigd op 04/02/2016 19:34:36 door Allard Keij
 
Randy vsf

Randy vsf

04/02/2016 19:53:14
Quote Anchor link
Je gebruikt prepared statements, dus dat zal wel goed zitten.
Wat voor software gebruik je? Heb je uiteindelijk toegang tot de losse files?
 
Thomas van den Heuvel

Thomas van den Heuvel

04/02/2016 20:03:20
Quote Anchor link
Quote:
Je gebruikt prepared statements, dus dat zal wel goed zitten.


Ayyy lmao.

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
<?php
$stmt
= $mysqli->prepare("INSERT INTO users (fbid,name,discountcode) VALUES ('$fbid', '$username', '" . randomCode(8) . "')");
?>


Hoe is dat een juist gebruik van prepared statements? Je injecteert rauwe, niet ge-escapete waarden in je statement, in plaats van deze te assignen via placeholders. Het netto effect van je bind_param() aanroepen is nul komma nul.

Nu kun je met deze query wellicht niet zoveel ongein uithalen (maar de vindingrijkheid van hackers is waarschijnlijk vele malen groter dan die van mij) maar ik vrees met grote vreze als de rest van je applicatie op dezelfde wijze omgaat met "prepared statements". Dit is in ieder geval niet de juiste manier.

Bonus ending: waar selecteer je de te gebruiken character encoding bij het maken van een connectie?
Gewijzigd op 04/02/2016 20:11:31 door Thomas van den Heuvel
 
Randy vsf

Randy vsf

04/02/2016 20:15:47
Quote Anchor link
Thomas van den Heuvel op 04/02/2016 20:03:20:
Quote:
Je gebruikt prepared statements, dus dat zal wel goed zitten.


Ayyy lmao.

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
<?php
$stmt
= $mysqli->prepare("INSERT INTO users (fbid,name,discountcode) VALUES ('$fbid', '$username', '" . randomCode(8) . "')");
?>


Hoe is dat een juist gebruik van prepared statements? Je injecteert rauwe, niet ge-escapete waarden in je statement, in plaats van deze te assignen via placeholders. Het netto effect van je bind_param() aanroepen is nul komma nul.

Nu kun je met deze query wellicht niet zoveel ongein uithalen (maar de vindingrijkheid van hackers is waarschijnlijk vele malen groter dan die van mij) maar ik vrees met grote vreze als de rest van je applicatie op dezelfde wijze omgaat met "prepared statements". Dit is in ieder geval niet de juiste manier.

Bonus ending: waar selecteer je de te gebruiken character encoding bij het maken van een connectie?


Deze site is een hel op de mobiel, en zag alleen de bind_param() staan, ging er vanuit dat het dan wel goed was.
 
Allard Keij

Allard Keij

04/02/2016 20:19:09
Quote Anchor link
Dat is tevens ook de reden dat ik hier kom. Zoals ik al aangeef, ik heb hier allemaal geen verstand van maar ik ben genoodzaakt gebruik te maken van PHP om mijn doel te bereiken binnen mijn app.

Kortom, geen idee of de code goed is ja of nee.

Ik weet alleen dat de code goed werkt, maar ik weet ook dat er 100den manieren zijn en ik er altijd langs pis.

Ik kan trouwens gebruik maken van txt bestanden. Dit zal wel altijd hetzelfde bestand zijn.
Gewijzigd op 04/02/2016 20:22:57 door Allard Keij
 
Randy vsf

Randy vsf

04/02/2016 20:21:26
Quote Anchor link
Je moet zoals thomas aangeeft de variablen in de query vervangen met markers.

Iets te lezen met een voorbeeld
 
Dennis WhoCares

Dennis WhoCares

04/02/2016 20:41:56
Quote Anchor link
Randy vsf op 04/02/2016 19:53:14:
Wat voor software gebruik je?

Ben ik ook wel benieuwd naar, ik doe obj-C met xCode voor iOS en ik kan gewoon json requests doen en daar een agent aan koppelen. (wel basic om te kijken of het van m'n app komt, maar toch)
 
Allard Keij

Allard Keij

04/02/2016 20:47:08
Quote Anchor link
Nou, ik heb mijn code aangepast en het werkt nog altijd :). Kortom ik neem aan dat het zo gefixed is en dus ook veilig is?

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
<?php
header('Access-Control-Allow-Origin: *');

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

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

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 (?,?,'" . randomCode(8) . "')");

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

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

$stmt->execute();

printf ($mysqli->insert_id);

$stmt->close();

$mysqli->close();
?>


Ik gebruik Construct 2 voor mijn apps. Ik ben helaas niet geboren om te programmeren en ik ben de juiste doelgroep voor software zoals dat. Drag en drop maar toch de vrijheid om het zo gek te maken als je maar wil.

Momenteel ben ik bezig met een app waar op het moment een Facebook login in zit, vervolgens ( nu dus veilig ) de gebruiker in een database zet met een Facebookid, waarmee ik dus ook de profielfoto kan gebruiken en de echte naam van de gebruiker. Vervolgens zit er een soort live feed in die dagelijks geupdate kan worden in een backend. Een comment system, een like system en noem maar op.

Alles werkt perfect maar ik wil het natuurlijk wel veilig maken. Ik ben al zo'n "Ach dat komt vast wel goed" persoon maar ik heb weinig zin in een scriptkiddie die straks heel mijn app naar de klote maakt door bijv. een klein stukje code als comment te posten waardoor heel de database op z'n kop gaat.

Thanks voor de hulp!
Gewijzigd op 04/02/2016 20:48:09 door Allard Keij
 
Ben van Velzen

Ben van Velzen

04/02/2016 21:22:12
Quote Anchor link
Je nieuwe script komt in de buurt, maar waarom prepare je de rest van je query ook niet gewoon? Dus ook de random code. Daarnaast: zie regels 23 t/m 26. De volgorde is raar, kan niet werken, en je maakt onnodig variabelen aan.
 
- SanThe -

- SanThe -

04/02/2016 21:25:05
Quote Anchor link
Dit: randomCode(8) geeft geen garantie op een unieke code.
 
Ben van Velzen

Ben van Velzen

04/02/2016 21:34:19
Quote Anchor link
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

Allard Keij

04/02/2016 21:40:57
Quote Anchor link
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

Dennis WhoCares

05/02/2016 08:17:19
Quote Anchor link
time() of microtime() zijn behoorlijk uniek? :D
 
Allard Keij

Allard Keij

11/02/2016 17:52:36
Quote Anchor link
Het is al weer even geleden maar ik test zojuist mijn code en zie nog altijd problemen.

Mijn huidige code is:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
<?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

Ben van Velzen

11/02/2016 18:03:27
Quote Anchor link
Gewoon POST gebruiken ipv GET?
 
- SanThe -

- SanThe -

11/02/2016 18:06:55
Quote Anchor link
6 stuks: (feedid, avatarid, username, userid, postid, comment)
VALUES
7 stuks: (?,?,?,?,?,?,?)
 
Allard Keij

Allard Keij

11/02/2016 18:27:11
Quote Anchor link
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 ).
Gewijzigd op 11/02/2016 20:13:44 door Allard Keij
 
Dennis WhoCares

Dennis WhoCares

11/02/2016 20:17:27
Quote Anchor link
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
Gewijzigd op 11/02/2016 20:25:02 door Dennis WhoCares
 
Allard Keij

Allard Keij

11/02/2016 20:25:47
Quote Anchor link
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.
Gewijzigd op 11/02/2016 20:26:26 door Allard Keij
 

Pagina: 1 2 volgende »



Overzicht Reageren

 
 

Om de gebruiksvriendelijkheid van onze website en diensten te optimaliseren maken wij gebruik van cookies. Deze cookies gebruiken wij voor functionaliteiten, analytische gegevens en marketing doeleinden. U vindt meer informatie in onze privacy statement.