Deeds101 / CYBR8420-project

3 stars 5 forks source link

Code Review #5 #39

Open Atmcalpine opened 7 months ago

Atmcalpine commented 7 months ago

Code review for the following files:

DoomDragoon commented 7 months ago

First drat for analysis on these two files

Code review

We chose to do an automated review because none of 
are group members are exceptionally good at coding and/or 
identifying vulnerabilities in raw code. Before even begining, 
our team knew this would be a challenge. Thus, auotmated review to address this challenge.
The code review software we used is called Synk. It integrates well with GitHub and
is easy to use and import multiple open source projects.
In additionj it is exceptionally good at analysis.
Not only does it identify potential vulnerabilites, it
performs a fix analysis based on the discoverya and resolution of
other open source projects it has scanned on github.

File Analysis: post/location.php

The file has a potential SQL injection vulnerabiilty. Lines 28-47 is a check to see if a file is attached for uploading directly into the query mysqli_query. This could give bad actors direct access to the database.

CWE-89

File Analysis: plugins/PHPMailer/src/POP3.php

The file has a potential privacy leak. Password data flows directly into an echo statement at line 396. Transit (API) and sotrage(server) should both be encrypted.

CWE-532

Atmcalpine commented 7 months ago

Love the write-up on the strategy. I would add in additional detail to address how we identified the files to be reviewed and the applicable CWEs to focus in on since those were specifically called out for the assignment. For example:

Review of ITFlow's modules/available files identified the following high risk files that should be evaluated for potential vulnerabilities based on prior security analysis performed as part of the CYBR8420 course:

The files were then split between team members who were then responsible for reviewing their designated files for vulnerabilities related to applicable CWEs (e.g., 79, 89, 532, 601, 614, 916 and 1004). These CWEs were identified based on the programming languages leveraged by ITFlow, prior security analyses performed as part of this course, and review of high risk vulnerabilities noted within the Snyk application.

We don't have to use this verbiage verbatim, but wanted to provide it as an example.

DoomDragoon commented 7 months ago

Had to change location.php to ajax.php as the code scan was picking up a false positive. Review of these files is complete.