image

Bijna 10.000 regels code pEp-privacymachine gecontroleerd

maandag 17 oktober 2016, 09:35 door Redactie, 13 reacties

Onderzoekers hebben de bijna 10.000 regels code van de pEp-privacymachine gecontroleerd, de software die volgens de gelijknamige stichting alle geschreven communicatie op internet moet versleutelen. De pEp (pretty Easy privacy) stichting wil encryptie zo gebruiksvriendelijk mogelijk maken.

Dit moet ervoor zorgen dat straks de geschreven communicatie van iedereen standaard is versleuteld. Om dit doel te bereiken ontwikkelt de stichting verschillende tools, zoals encryptietools voor Android en Outlook. Deze software draait op de pEp-engine. Zowel voor transparantie als veiligheid is er vorig jaar augustus een audit van de code gestart, bestaande uit bijna 10.000 regels van de programmeertaal C. Het Duitse Sektion Eins voerde de audit uit, waarbij regel voor regel werd gecontroleerd.

In totaal werden er zeven kwetsbaarheden met de beoordeling "medium" gevonden en vier die als "high" werden aangeduid, alsmede verschillende andere zaken. De beveiligingslekken zijn inmiddels in de software verholpen. Tevens legde de audit verschillende ontwerpbeslissingen bloot. Niet per definitie fouten, maar wel zaken die tot verwarring of toekomstige problemen konden leiden.

De ontwikkelaars van de pEp-stichting merken op dat het hier om een proces gaat, waarbij de audits continu zullen worden herhaald. "Dit biedt geen 100% garantie op code zonder fouten, maar het houdt wel in dat onafhankelijke specialisten en wijzelf alles zullen doen dat menselijk mogelijk is om dat doel te bereiken", zegt Hernani Marques van de pEp-stichting. Het auditrapport en aanvullende documenten zijn nu te downloaden.

Reacties (13)
17-10-2016, 12:43 door [Account Verwijderd] - Bijgewerkt: 18-10-2016, 17:03
[Verwijderd]
17-10-2016, 13:12 door Erik van Straten
17-10-2016, door Rinjani: In een paar minuten even rondkijken vind ik een potentiële bug in wat de laatste versie van stringpair.c lijkt te zijn (https://cacert.pep.foundation/trac/browser/src/stringpair.c [@redactie: soms werken hyperlinks niet]):

DYNAMIC_API stringpair_t * new_stringpair(const char *key, const char *value)
  [...]
  assert(key);
  [...]
  pair->key = strdup(key);
  [...]

Die strdup(key) aanroep crasht wanneer key een null pointer is (waarop niet afdoende wordt gecontroleerd want alleen assert statement).

Checken of de parameter "key" een null-pointer bevat is leuk, maar wat als "key" ongelijk nul is maar naar een vrijgegeven of ander ongeïnitialiseerd stuk geheugen wijst? Het risico daarvan is meestal groter dan van een null-pointer dereference.

In dit soort situaties is het gebruikelijk om de aanroepende functie (of eerder) ervoor verantwoordelijk te maken dat "key" fatsoenlijk is geïnitaliseerd (user supplied input in een zo vroeg mogelijk stadium valideren). Ik heb overigens niet naar andere sources gekeken, d.w.z. waar deze functie wordt, of kan worden, aangeroepen, en weet dus niet of die controles plaatsvinden en "waterdicht" zijn.
17-10-2016, 13:18 door krista_g - Bijgewerkt: 17-10-2016, 13:19
Rinjani: Nee, je hebt helemaal gelijk. Opgelost - hartstikke bedankt :)
17-10-2016, 13:19 door Anoniem
Als die functie altijd wordt aangeroepen met een key die niet NULL kan zijn dan is er niks aan de hand.
Dus je moet dan verder gaan zoeken naar de aanroepers van die functie.
17-10-2016, 13:34 door krista_g
Door Erik van Straten:
17-10-2016, door Rinjani: In een paar minuten even rondkijken vind ik een potentiële bug in wat de laatste versie van stringpair.c lijkt te zijn (https://cacert.pep.foundation/trac/browser/src/stringpair.c [@redactie: soms werken hyperlinks niet]):

DYNAMIC_API stringpair_t * new_stringpair(const char *key, const char *value)
  [...]
  assert(key);
  [...]
  pair->key = strdup(key);
  [...]

Die strdup(key) aanroep crasht wanneer key een null pointer is (waarop niet afdoende wordt gecontroleerd want alleen assert statement).

Checken of de parameter "key" een null-pointer bevat is leuk, maar wat als "key" ongelijk nul is maar naar een vrijgegeven of ander ongeïnitialiseerd stuk geheugen wijst? Het risico daarvan is meestal groter dan van een null-pointer dereference.

In dit soort situaties is het gebruikelijk om de aanroepende functie (of eerder) ervoor verantwoordelijk te maken dat "key" fatsoenlijk is geïnitaliseerd (user supplied input in een zo vroeg mogelijk stadium valideren). Ik heb overigens niet naar andere sources gekeken, d.w.z. waar deze functie wordt, of kan worden, aangeroepen, en weet dus niet of die controles plaatsvinden en "waterdicht" zijn.

Natuurlijk - we zijn voorzichtig met dit soort input-checking respectievelijk initialisatie; anders zijn null checks waardeloos :) The assert-without-if slipped through here, though, so small or not, it's a good catch. We appreciate it.
17-10-2016, 14:03 door [Account Verwijderd] - Bijgewerkt: 17-10-2016, 15:19
[Verwijderd]
17-10-2016, 14:49 door Anoniem
Iedereen leest Security.nl :)

https://twitter.com/pEpCouncil/status/787972918177329152
17-10-2016, 15:12 door Anoniem
Goed dat het snel gefixt wordt, maar die audit... (over het hoofd gezien? :S)
17-10-2016, 15:26 door Anoniem
I'm not happy about this.

new_ functions in pEp engine are considered constructors. They have to deliver a pointer to the constructed object, or NULL on out of memory.

Unfortunately, the constructor new_stringpair() is designed to take non-optional parameters (practically all other constructors are taking optional parameters only). This is, because a stringpair of less than two strings does not make sense semantically.

And here we go: it is impossible to find a correct implementation, which keeps the original assumptions, and additionally delivers a parameter error if parameters are missing (are NULL).

The “fix” we have now is changing the behaviour of the function. It's not any more delivering NULL on out of memory, but NULL on any error (which includes parameters missing).

This is generating a bug in pgp_gpg.c:1289 for example.

I think, we have to revert that for good reasons.
17-10-2016, 16:41 door Erik van Straten
@Rinjani: it may not be a good idea to discuss potential security vulnerabilities on an open site like this, as bad guys may be reading this too and it takes time before tested fixes are in use by (security-concious) end users.

In het Nederlands: het is wellicht geen goed idee om potentiële beveiligingskwetsbaarheden te bespreken op een open site als deze, omdat kwaadwillenden mogelijk meelezen en het tijd kost voordat geteste fixes gebruikt worden door (security serieus nemende) eindgebruikers.

@krista_g: thanks for your answer! I'm going to find time (no idea how yet ;-) to look at pEp! The world definitely needs signed e-mails to help prevent phishing, while end-to-end encryption is important in some cases (encrypted mail also poses new security risks).
17-10-2016, 16:57 door [Account Verwijderd] - Bijgewerkt: 17-10-2016, 17:13
[Verwijderd]
17-10-2016, 17:25 door Erik van Straten
Door Rinjani:
Door Erik van Straten:In het Nederlands: het is wellicht geen goed idee om potentiële beveiligingskwetsbaarheden te bespreken op een open site als deze, omdat kwaadwillenden mogelijk meelezen en het tijd kost voordat geteste fixes gebruikt worden door (security serieus nemende) eindgebruikers.

De software is al in gebruik? Oeps, sorry. Dat had ik me niet gerealiseerd...
Heel begrijpelijk, zelf ben ik ook wel eens onachtzaam geweest in mijn enthousiasme om zaken te helpen verbeteren!

Helaas is dit, vermoed ik, ook een van de redenen dat maar weinig mensen de moeite nemen om open source code te auditten: vaak twijfel je of iets echt een kwetsbaarheid is, en wilt dat met anderen overleggen zonder de auteurs lastig te vallen met zaken waar zij mogelijk al aan gedacht hebben. Het is dan heel prettig als je dit eerst met anderen kunt bespreken, maar juist bij security issues introduceer je mogelijk risico's - want die anderen kunnen heel andere bedoelingen hebben dan jij.
17-10-2016, 19:31 door Anoniem
Door Erik van Straten: @Rinjani: it may not be a good idea to discuss potential security vulnerabilities on an open site like this, as bad guys may be reading this too and it takes time before tested fixes are in use by (security-concious) end users.

In het Nederlands: het is wellicht geen goed idee om potentiële beveiligingskwetsbaarheden te bespreken op een open site als deze, omdat kwaadwillenden mogelijk meelezen en het tijd kost voordat geteste fixes gebruikt worden door (security serieus nemende) eindgebruikers.

@krista_g: thanks for your answer! I'm going to find time (no idea how yet ;-) to look at pEp! The world definitely needs signed e-mails to help prevent phishing, while end-to-end encryption is important in some cases (encrypted mail also poses new security risks).
Ik stel daarom voor dat je de makers via Twitter met een DM uitlegt wat aan de hand is. Een open discussie is inderdaad mooi, maar heeft inderdaad zijn schaduwzijden. Gr.Peter V.
Reageren

Deze posting is gelocked. Reageren is niet meer mogelijk.