Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 23 Jun 2023 13:22:17 +0200
From: Solar Designer <solar@...nwall.com>
To: oss-security@...ts.openwall.com
Cc: Jyoti Raval <jenyraval@...il.com>
Subject: Re: Open Source Tool | MPT: Pentest In Action!

Hi,

For those wondering why this got through moderation, it's because we do
have a relevant item among the list content guidelines:

https://oss-security.openwall.org/wiki/mailing-lists/oss-security#list-content-guidelines

"Occasional announcements of Open Source security tools (and relevant
features of non-security tools) are acceptable, but only for initial
announcements and major updates (not for minor updates).  Especially
desirable are news on tools/features aimed to enhance security of other
Open Source software."

Unfortunately, this particular tool doesn't appear to be "aimed to
enhance security of other Open Source software".

On Thu, Jun 22, 2023 at 06:05:14PM +0530, Jyoti Raval wrote:
> Managing Pentest (MPT: Pentest In Action) [image: HITBSecConf HITB2022SIN]
> <https://conference.hitb.org/hitbsecconf2022sin/session/mpt-pentest-in-action/>

This isn't a topic for oss-security.  But per the above, an Open Source
security tool announced for the first time nevertheless is.

> Github - https://github.com/jenyraval/MPT

Also, security issues in an Open Source tool are on topic here.  Let's
see what we have for this one:

login.php:
      $myusername = mysqli_real_escape_string($db,$_POST['username']);
      $mypassword = mysqli_real_escape_string($db,$_POST['password']);

      $sql = "SELECT id FROM login WHERE username = '$myusername' and password = '$mypassword'";
      $result = mysqli_query($db,$sql);

No use of prepared statements, instead relying solely on escaping.
Given that the specialized escaping function is used, this is supposed
to work, but I think is a higher risk than prepared statements.  I'll
spare this one from an OVE ID assignment, although I do think it's
unjustified risk exposure.

Plaintext password storage.  OVE-20230623-0001

Password comparison potentially vulnerable to remote timing attack
(depending on undocumented MySQL server internal workings, which isn't
something to rely upon for security).  OVE-20230623-0002

live_edit.php:
$input = filter_input_array(INPUT_POST);
if ($input['action'] == 'edit') {
$update_field='';
if(isset($input['status'])) {
$update_field.= "status='".$input['status']."'";
}
if($update_field && $input['id']) {
$sql_query = "UPDATE issuedetails SET $update_field WHERE id='" . $input['id'] . "'";
mysqli_query($db, $sql_query) or die("database error:". mysqli_error($conn));

(Yes, the lack of indentation is in the original.)

Apparently, no escaping nor filtering is actually performed here, and
also no use of prepared statements.  Likely (post-authentication?) SQL
injection possibility.  OVE-20230623-0003

Per PHP documentation, filter_input_array() "is useful for retrieving
many values without repetitively calling filter_input()."  As optional
second argument (missing here), it'd accept an actual filter.  The
default is FILTER_DEFAULT, just like for filter_input(), the
documentation for which says: "If omitted, FILTER_DEFAULT will be used,
which is equivalent to FILTER_UNSAFE_RAW.  This will result in no
filtering taking place by default."

Should PHP possibly want to deprecate usage of filter_input() and
filter_input_array() without a filter specified, as this provides a
false sense of security?

I could be missing something here - the above is based solely on my
current reading of PHP documentation.

Throughout the MPT codebase, data already in the database is trusted not
to cause SQL injections nor XSS.  As I'm not seriously auditing this
codebase, I did not check the data flow, but I suspect that no
validation sufficient against both of these risks takes place on
entering the data into the database.  Even if
mysqli_real_escape_string() is used, which it appears to be in many
places, this should only prevent SQL injection on the INSERT/UPDATE
itself, but not on subsequent reusage of the string SELECT'ed back from
the database in further SQL queries.  It also does not prevent XSS.
Let's call this OVE-20230623-0004, although it could as well be two IDs.

I think that's enough to turn the thread into something relevant here -
especially the question on PHP's filter_input*() and its hardening.

Alexander

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.