[PHP] mail() verstuurd niet alle variabelen en tekst

[Jasper_S1985]

Mijn vraag

Ik moet voor mijn werk een mailformuliertje laten versturen via een website en ben hier nu mee aan het bouwen. De e-mail wordt prima verstuurd alleen komen niet alle variabelen in het mailtje te staan.

De enige regels die in het mailtje komen te staan zijn:
- From: $achternaam \n
- Interactieve tv: $interactievetv \n
- Message: $opmerkingen";

De rest wordt gewoon niet weergegeven in de mail. Zowel de tekst als de variabelen worden genegeerd. Wat zie ik over het hoofd?

Hieronder staat het bestand dat de variabelen aanmaakt en het mailtje verstuurd. Ik heb al gechecked dat alle namen en variabelen kloppen met het formulier.

PHP:

$geslacht = $_POST['geslacht'];
$achternaam = $_POST['achternaam'];
$voorletters = $_POST['voorletters'];
$caiwayklant = $_POST['klant'];
$interactievetv = $_POST['interactieve-tv'];
if(isset($_POST['tvplus'])) $tvplus = $_POST['tvplus'];
if(isset($_POST['opmerkingen'])) $message = $_POST['opmerkingen'];
$formcontent="
Geslacht: $geslacht \n
Voorletters: $voorletters \n
From: $achternaam \n
Caiway klant: $klant \n
Interactieve tv: $interactievetv \n
Extra zenderpakketten: $tvplus;
Message: $opmerkingen";
$recipient = "info@gewijzigdvoortweakers";
$subject = "Contact Form";
$mailheader = "From: $klant \r\n";
mail($recipient, $subject, $formcontent, $mailheader) or die("Error!");
echo "dank!";

[ Voor 6% gewijzigd door RobIII op 16-05-2019 10:40 . Reden: Code tags gefixed ]

[reddevil]

Behalve dat je niet direct dingen uit de post moet halen zonder te checken of het valid is (en andere security dingen), zijn de variables '$klant' en '$opmerkingen' undefined. Gebruik een goede IDE en die laat dat meteen zien

Oh en misschien de php warnings/errors aanzetten, dan zou dat ook op je scherm getoond moeten worden.

[ Voor 18% gewijzigd door reddevil op 16-05-2019 10:36 ]

[Jasper_S1985]

Maar hoe komt het dan dat hij ook de tekst "Extra zenderpakketten" etc niet weergeeft want dat lijkt me gewoon platte tekst...

[RobIII]

Omdat je 99.99% zeker ergens iets niet correct escaped. En je formulier is zéér waarschijnlijk vatbaar voor mail header injection. Doe jezelf, en je baas, een lol en verdiep je even in de materie (doe een cursus/tutorial PHP) of besteed 't uit aan een ontwikkelaar. Want, with all due respect, dit is een disaster waiting to happen.

[ Voor 18% gewijzigd door RobIII op 16-05-2019 11:18 ]

[reddevil]

Stel je zegt:

code:

$a = "mies";
$uitvoer = $b . ' noot ' . $a;

Wat zou $uitvoer dan moeten zijn als de compiler geen idee heeft wat '$b' is? Met undefined variables kan van alles gebeuren, dus fix je warnings ... en je escaping zoals hierboven aangegeven is.

[ Voor 27% gewijzigd door reddevil op 16-05-2019 10:43 ]

[RobIII]

reddevil schreef op donderdag 16 mei 2019 @ 10:42:
Stel je zegt:

code:

$a = "mies";
$uitvoer = $b . ' noot ' . $a;

Wat zou $uitvoer dan moeten zijn als de compiler geen idee heeft wat '$b' is?

Dat geeft een "Notice: Undefined variable: b" maar geeft als output gewoon:

 noot mies

YAY PHP!

@ShitHappens hieronder:

ShitHappens schreef op donderdag 16 mei 2019 @ 10:45:
Het setten van $formcontent gaat dan een tijdje goed, tot de eerste undefined error ($klant) opspeelt. Dan worden verdere handelingen afgekapt

Nee dus Er wordt een NOTICE ge-emit en PHP gaat vrolijk door. Afhankelijk van o.a. je error_reporting en display_errors settings zie je die notice dus nooit en zie je gewoon de output waar op de plekken van de onbekende variabelen gewoon niets staat.

[ Voor 62% gewijzigd door RobIII op 16-05-2019 10:58 ]

[ShitHappens]

Jasper_S1985 schreef op donderdag 16 mei 2019 @ 10:39:
Maar hoe komt het dan dat hij ook de tekst "Extra zenderpakketten" etc niet weergeeft want dat lijkt me gewoon platte tekst...

Het setten van $formcontent gaat dan een tijdje goed, tot de eerste undefined error ($klant) opspeelt. Dan worden verdere handelingen afgekapt, gok ik (PHP is alweer 10 jaar geleden voor mij).
Dus die $klant op regel 12 zou dan al $caiwayklant moeten zijn? En de situatie dat $tvplus en $opmerkingen undefined kunnen zijn zal ook afgehandeld moeten worden.
En inderdaad don't trust user input, alle variabelen even schoonmaken, escapen en checken op geldigheid.

[ Voor 3% gewijzigd door ShitHappens op 16-05-2019 10:46 ]

[Groax]

Ik ben even lief.
Hier heb je een werkend script met wat meer veiligheid.

Kleine tip
Voeg dit ook toe aan een Database. mocht de mail niet verstuurd worden dan heb je de form wel.

Bronnen:
htmlspecialchars - https://www.w3schools.com/php/php_form_validation.asp
Short If - https://davidwalsh.name/php-ternary-examples

Extra info:
https://www.w3schools.com/php/php_form_url_email.asp

PHP:

//  Check of is $_POST gevuld is.
if (!empty($_POST)) {
    (isset($_POST['geslacht']) ? $geslacht=htmlspecialchars($_POST['geslacht']) : $geslacht='');
    (isset($_POST['achternaam']) ? $achternaam=htmlspecialchars($_POST['achternaam']) : $achternaam='');
    (isset($_POST['voorletters']) ? $voorletters=htmlspecialchars($_POST['voorletters']) : $voorletters='');
    (isset($_POST['klant']) ? $caiwayklant=htmlspecialchars($_POST['klant']) : $caiwayklant='');
    (isset($_POST['interactieve-tv']) ? $interactievetv=htmlspecialchars($_POST['interactieve-tv']) : $interactievetv='');
    (isset($_POST['tvplus']) ? $tvplus=htmlspecialchars($_POST['tvplus']) : $tvplus='');
    (isset($_POST['opmerkingen']) ? $opmerkingen=htmlspecialchars($_POST['opmerkingen']) : $opmerkingen='');

    $formcontent = "Geslacht: {$geslacht }\n
    Voorletters: {$voorletters }\n
    From: {$achternaam }\n
    Caiway klant: {$caiwayklant }\n
    Interactieve tv: {$interactievetv }\n
    Extra zenderpakketten: {$tvplus};
    Message: {$opmerkingen}";

    $recipient = "info@gewijzigdvoortweakers";
    $subject = "Contact Form";
    $mailheader = "From: $caiwayklant \r\n";
    mail($recipient, $subject, $formcontent, $mailheader) or die("Error!");
    echo "dank!";


//    echo nl2br($formcontent);
}

Het is echt belangrijk dat je de variables goed definieert. PHP gaat anders gewoon kapot.

$caiwayklant = $_POST['klant']; was $klant...
$message = $_POST['opmerkingen'] was $opmerkingen...

Hier ging die kapot.

[Jasper_S1985]

Bedankt!

[CurlyMo]

Jasper_S1985 schreef op donderdag 16 mei 2019 @ 12:16:
Bedankt!

Wat nu als het contact formulier opeens niet meer werkt. Snap jij het dan om het op te kunnen lossen? Ga je dat bijtijds doorhebben? Wegen de kosten van gemiste klanten op tegen nu iemand inhuren of een commerciële mailing tool gebruiken waar je ondersteuning van mag verwachten?

Vind Caiway het sowieso wel een prettig idee dat jij bij een enorme potentiële klantenkring met kennis laat blijken dat er knullige ICT oplossingen worden toegepast of op onderdelen onkundige mensen werken?

[ Voor 20% gewijzigd door CurlyMo op 16-05-2019 12:43 ]

[RobIII]

moese schreef op donderdag 16 mei 2019 @ 12:10:
Ik ben even lief.

Nou... je intentie is goed en lief hoor Maar je hebt 't probleem niet opgelost (en mogelijk zelfs groter gemaakt). Allereerst heb je TS nu gewoon kant-en-klare, copy/paste code gegeven. En daar zit probleem 1:

Give a man a fish and feed him for a day. Teach a man how to fish and feed him for a lifetime.

Probleem 2 is dat je nu een schijnveiligheid creeërt door te zeggen dat 't veiliger is; dat is het niet. Het is nog steeds vatbaar voor mailheader injection.
Probleem 3 is dat TS niets heeft geleerd van hoe PHP omgaat met errors/notices etc. en hoe je dat regelt. Precies de reden waarom ik TS wat linkjes gaf. En tenslotte is probleem 4 dat je nu de luiheid van TS beloont met een kant-en-klare oplossing waarmee hij/zij ons de volgende keer wéér zal behandelen als afhaalchinees. Je bent dus een 'enabler'

Nogmaals; je intentie was/is goed natuurlijk en ik zeg niet dat TS ons nu als afhaalchinees ziet, maar je ziet wel waar ik vandaan kom hoop ik?

...en ik zie net dat ik je dit niet voor 't eerst vertel

[ Voor 8% gewijzigd door RobIII op 16-05-2019 13:20 ]

[Groax]

RobIII schreef op donderdag 16 mei 2019 @ 13:16:
Nogmaals; je intentie was/is goed natuurlijk en ik zeg niet dat TS ons nu als afhaalchinees ziet, maar je ziet wel waar ik vandaan kom hoop ik?

Ik snap je maar ik geef een verbetering op zijn eigen code.
De veiligheid van de mailheader injection is zijn eigen verantwoordelijkheid als hij deze negeert. Aangezien dit eerder is aangegeven.

Ik zelf sla alles op in een database met een "Im not a Robot".

[DataGhost]

moese schreef op donderdag 16 mei 2019 @ 13:30:
[...]

De veiligheid van de mailheader injection is zijn eigen verantwoordelijkheid als hij deze negeert. Aangezien dit eerder is aangegeven.

Dan moet je IMO je post niet beginnen met "wat meer veiligheid" als je dat niet geeft. Voor een hoop zaken zijn vrijwel kant-en-klare oplossingen in de vorm van libraries die je ook dient te gebruiken. Bijvoorbeeld voor gedoe met datums/tijden/tijdzones, maar e-mail is daar zeker eentje van. Als je zelf wat aan gaat prutten zal het wel kunnen werken, maar je maakt hier echt iets wat op een gegeven moment problemen gaat geven.

@Jasper_S1985: Je bent nog steeds vatbaar voor header injection, zoals @RobIII al aangaf. Ik heb niet eens gekeken naar de rest van het script, dus wellicht zijn er nog meer aanvalsvectoren. Maar doe jezelf en je werk/opdrachtgever een lol en gebruik een uitontwikkelde library, waar heel veel mensen heel erg lang over hebben nagedacht, of besteed het uit aan iemand die dit professioneel doet. Er zijn behoorlijk wat bots die websites afstruinen naar contactformulieren en daar volledig geautomatiseerd wat dingen op afvuren om te kijken of ze er spam mee kunnen versturen (en dat kan met dit script). Op het moment dat het formulier gevonden is zal dat zoveel mogelijk misbruikt gaan worden, met als gevolg dat de mailserver die gebruikt wordt op allerlei blacklists komt, in het gunstigste geval. Het kan er echter ook voor zorgen dat de hele website van het net geknikkerd wordt door de hoster, of de verbinding wordt afgesloten, al dan niet vergezeld van facturen van partijen die het probleem op hebben moeten lossen.

[Jasper_S1985]

Het is eigenlijk alleen maar een formuliertje die door ons zelf ingevuld wordt en niet door klanten

[AW_Bos]

Dan alsnog vind ik dat je de zaakjes goed op orde moet hebben.

Ik zelf gebruik de library van PHPmailer om mijn mails mee te versturen. Deze werkt echt super gemakkelijk en is ook zeer flexibel in te stellen qua aflever methodes.

[DataGhost]

Dan nog moet je er enorm voor zorgen dat 'ie ook niet bereikbaar is (dmv .htaccess password ofzo) en heb je alsnog kans dat je mail uberhaupt niet aankomt, redelijk wat mailservers markeren mail van de standaard php mail-functie als spam.

En nu is het nog "alleen intern" "dus niet zo belangrijk qua security" maar als je nogmaals zoiets moet maken ben je al gauw geneigd dit te hergebruiken en ben je allang vergeten dat het eigenlijk niet geschikt is om in productie te draaien. Het zal ook niet de eerste keer zijn dat het gebruiksdoel wordt aangepast en "iedereen" het moet gaan gebruiken, en het "werkt nu toch?"

[ Voor 44% gewijzigd door DataGhost op 16-05-2019 14:18 ]

[Wim-Bart]

Wel grappige code als zijn baas beslist dat de ingevoerde data ook nog in een database moet. Code kan nog steeds gebruikt worde voor SQL Injection.

[Jasper_S1985]

Zit niks database gerelateerd aan vast

[DataGhost]

Wim-Bart schreef op donderdag 16 mei 2019 @ 15:20:
Wel grappige code als zijn baas beslist dat de ingevoerde data ook nog in een database moet. Code kan nog steeds gebruikt worde voor SQL Injection.

Dat is iets wat je pas op het allerlaatste moment doet. Als je op de juiste manier gebruik maakt van prepared statements hoef je de input helemaal niet te behandelen, en anders pas je de juiste escaping pas op het laatste moment toe. Er is geen enkele reden om database-escaping te gaan doen op data die daarna niet naar een database gaat. Mensen die alle input maar gaan sql-escapen just in case, da\'s pas irritant. Net als als een kip zonder kop overal htmlspecialchars gebruiken "omdat iemand zei dat dat veilig was" terwijl de output plaintext is en geen html. Daarmee geef je eigenlijk aan dat je alleen maar ooit een truukje geleerd hebt maar niet weet waarom en wanneer je het moet gebruiken.

[ Voor 7% gewijzigd door DataGhost op 16-05-2019 17:16 ]

[Groax]

DataGhost schreef op donderdag 16 mei 2019 @ 17:13:

Net als als een kip zonder kop overal htmlspecialchars gebruiken "omdat iemand zei dat dat veilig was" terwijl de output plaintext is en geen html. Daarmee geef je eigenlijk aan dat je alleen maar ooit een truukje geleerd hebt maar niet weet waarom en wanneer je het moet gebruiken.

Dus ik heb een knap trucje ontdekt? Dat er overal htmlspecialchars wordt gebruikt in dat stukje code is omdat ik niet weet wat daar aan de hand is en welke velden er daadwerkelijk worden ingevuld.
Al had ik ook htmlspecialchars($formcontent) kunnen doen en dat had ook gewerkt.

Ook in een plaintext input text veld kan je html, javascript of wat dan ook mee sturen.

Hoe zou jij het hebben opgelost dan?

[Voutloos]

moese schreef op donderdag 16 mei 2019 @ 19:55:
ik niet weet wat daar aan de hand is

Juist. Dus daar had iedereen het bij kunnen laten.

Als er bewust enige structuur aanwezig was en expliciete keuzes gemaakt waren, had code de content type gedicteerd en had je (lees: men) wel kunnen weten welke escaping relevant was.

Allemaal goede bedoelingen, maar door het maar half te fixen, met de gegeven code style, verlaag je je meer tot het niveau dan dat je goede code uit draagt. Met in deze het risico dat het 1 op 1 overgenomen wordt, waarna er weer een eeuwige mail() beerput rondzwerft.

Overigens zou mailI() ook niet een deze vorm moeten bestaan, het 3e argument had minstens array only moeten zijn met Exception zodra er iets met header eindes (\r\n) gedaan wordt.

moese schreef op donderdag 16 mei 2019 @ 19:55:
Hoe zou jij het hebben opgelost dan?

Niet met dit vertrekpunt. Dus stap 1 is <ctrl+a>,<del>. Noem het desnoods achteraf maar een prototype. Mooi, nu terug naar het begin. Wat wil je bereiken? 1 geadresseerde, 1 vast formaat, wel of geen html mail? Requirements helder: Dan heb je meer houvast om een passende tutorial in dit geval te zoeken. Hierbij is het negatief als tutorial meer dan 10 jaar oud is en positief als er een recente, populaire library gebruikt wordt.

[ Voor 3% gewijzigd door Voutloos op 16-05-2019 20:44 ]

[DataGhost]

moese schreef op donderdag 16 mei 2019 @ 19:55:
[...]


Dus ik heb een knap trucje ontdekt? Dat er overal htmlspecialchars wordt gebruikt in dat stukje code is omdat ik niet weet wat daar aan de hand is en welke velden er daadwerkelijk worden ingevuld.
Al had ik ook htmlspecialchars($formcontent) kunnen doen en dat had ook gewerkt.

Je hebt de subtiele hint niet gezien of begrepen(&quot;), waarschijnlijk juist vanwege misbruik van die functie. Weet je wel wat htmlspecialchars doet, en waarom? Misschien is het handig om ook de documentatie te lezen, hoewel daar niet expliciet staat in welke situaties je het niet hoeft te gebruiken. Het is, net als het excessieve gebruik van mysql_real_escape_string, vroeger de wereld in geslingerd door luie "programmeurs" met weinig kennis, die code injection wel zagen als probleem, maar er eigenlijk niet tegen wilden coden. Het gevolg is dat gewoon alle input voorzien werd van die onzin, en het was niet meer mogelijk om te injecten. Daar zijn vervolgens duizenden tutorials mee geschreven, en het is klakkeloos overgenomen als bijbelse waarheid door volslagen beginners die daarna nooit meer hebben teruggekeken naar "waarom moet ik dit hier gebruiken?" en helemaal nooit het beoogde doel en de side-effects van de code hebben begrepen. In veel gevallen ging het "redelijk goed" maar je ziet nog steeds op heel veel plekken artifacts ervan.

Wat jij doet is vergelijkbaar met "Nee joh, je hoeft geen rijles te nemen. Je moet alleen maar voordat je gaat rijden je ruitenwissers aanzetten en dan komt alles vanzelf goed." zonder daarbij het waarom uit te leggen (of te weten). Dan rijdt de TS op een prachtige dag na een paar minuten een andere auto tot puin en dan is het "ja maar ik had de ruitenwissers gewoon aan hoor!"

Htmlspecialchars pas je pas toe op een string vlak voordat 'ie naar een browser gaat. Een database doet helemaal niks met html dus heeft daar ook geen problemen mee. Dus pas op het moment dat je je waarde uit de database haalt en naar de browser stuurt gooi je er htmlspecialchars omheen. Als je dat niet netjes doet en gewoon al je input html- en sql-escapet zonder naar het doel te kijken heb je al gauw iets als
Input: De overnachting is geboekt voor "Hotel 't Slapertje" op 23 -> 26 mei.
Output: De overnachting is geboekt voor &quot;Hotel \&#039;t Slapertje&quot; op 23 -&gt; 26 mei.

Ook in een plaintext input text veld kan je html, javascript of wat dan ook mee sturen.

Ik heb nergens "plaintext input" gezegd, ik zei dat de output plaintext is. In je voorbeeld hierboven komt de input helemaal niet in een browser terecht, maar in een e-mail, die text/plain is en niet text/html. Daarin hebben de "html special chars" helemaal geen betekenis en maak je het alleen maar minder leesbaar voor de ontvangende partij, als zulke tekens toevallig ergens in de input zitten. Wat de ontvanger te zien krijgt zal dus dichter bij bovenstaande voorbeeld-output dan bij de voorbeeld-input zitten. Als jij "<script>gevaarlijkecode</script>" in de input hebt en geen htmlspecialchars toepast, ziet de ontvanger alsnog "<script>gevaarlijkecode</script>" in het mailtje staan, zonder dat die code wordt uitgevoerd. Met jouw htmlspecialchars eroverheen ziet de ontvanger dan "&lt;script&gt;gevaarlijkecode&lt;/script&gt;", dat doet nog steeds niks maar ziet er net een stapje minder begrijpelijk uit voor de ontvangende partij. Op geen enkel moment doe je een echo of print met een van de input-variabelen. Netto is de "veiligheid van het mailscript" dus met precies 0 toegenomen. Header injection in de mail, wat het grootste issue was in dat script en het is een spam-script verandert, is namelijk nog steeds mogelijk.

Hoe zou jij het hebben opgelost dan?

Ik zou het hebben opgelost door een fatsoenlijke mail-library te gebruiken. Daar stop je namelijk ontvanger, onderwerp en body in en de library zorgt ervoor dat deze gevalideerd worden en past eventueel noodzakelijke input-transformaties toe. Wat betreft de extra headers, die worden een voor een aan de library gegeven welke daar alle noodzakelijke checks op toepast zodat header injection ook niet meer mogelijk is. En tadaa, de mail wordt op een veilige manier verstuurd.

[ Voor 4% gewijzigd door DataGhost op 17-05-2019 07:06 ]

[reddevil]

moese schreef op donderdag 16 mei 2019 @ 12:10:

PHP:

    (isset($_POST['geslacht']) ? $geslacht=htmlspecialchars($_POST['geslacht']) : $geslacht='');

Ik stopte al met lezen bij die regel, d'r zit zoveel in waarvan ik dacht: "da's niet mooi" (in de breedste context van 'mooi')...

[ Voor 9% gewijzigd door reddevil op 17-05-2019 09:21 ]

[Jasper_S1985]

reddevil schreef op vrijdag 17 mei 2019 @ 09:21:
[...]


Ik stopte al met lezen bij die regel, d'r zit zoveel in waarvan ik dacht: "da's niet mooi" (in de breedste context van 'mooi')...

Fijn dat je daarna toch nog dacht.. goh laat ik eens een zinvolle bijdrage geven aan dit topic.

[Groax]

Jasper_S1985 schreef op vrijdag 17 mei 2019 @ 12:19:
[...]

Fijn dat je daarna toch nog dacht.. goh laat ik eens een zinvolle bijdrage geven aan dit topic.

Dit ook echt...