Merge branch 'reflection_bottlenecks' into 'master'

[ssproject1617.git] / report / v2_authentication.tex

diff --git a/report/v2_authentication.tex b/report/v2_authentication.tex
index d58f562..d2407c4 100644 (file)
--- a/report/v2_authentication.tex
+++ b/report/v2_authentication.tex
@@ -1,5 +1,5 @@
 
 

-\begin{enumerate}[label={2.\arabic*}]
+\begin{enumerate}[label={V2.\arabic*}]

 
 \item
 \fail{}
 
 \item
 \fail{}


@@ -15,15 +15,19 @@ public (Principle of complete mediation).
 \end{result}
 
 \item
 \end{result}
 
 \item

-\pass{}
+\fail{}

 Verify that forms containing credentials are not filled in by
 the application. Pre-filling by the application implies that
 credentials are stored in plaintext or a reversible format,
 which is explicitly prohibited.
 
 \begin{result}
 Verify that forms containing credentials are not filled in by
 the application. Pre-filling by the application implies that
 credentials are stored in plaintext or a reversible format,
 which is explicitly prohibited.
 
 \begin{result}

-No credentials (that should not be stored in plain text) are ever filled in by
-the application.
+No credentials that come from the database are pre-filled by the application.
+However, in some forms, the application pre-fills password fields from the
+request's POST data. This is not necesarry.\footnote{This issue was actually
+overlooked when auditing manually, and was found when running the Fortify tool.
+In the initial audit, we only ensured that no internal information (from the
+database) was leaked in this way.}

 \end{result}
 
 \setcounter{enumi}{3}
 \end{result}
 
 \setcounter{enumi}{3}


@@ -48,10 +52,10 @@ attackers cannot log in.
 
 \begin{result}
     The input to various forms is not sanitized at all. This makes the implementation
 
 \begin{result}
     The input to various forms is not sanitized at all. This makes the implementation

-    in (not only) \texttt{Users::find} vulnerable to SQL injections. The login form
-    is also vulnerable. Any user can execute arbitrary SQL code from the username field.
+    in (not only) \code{Users::find} vulnerable to \SQL{} injections. The login form
+    is also vulnerable. Any user can execute arbitrary \SQL{} code from the username field.

     The following example code can submitted as username in the login form, which
     The following example code can submitted as username in the login form, which

-    will set the password of the \texttt{admin} user to \texttt{s3cret}:
+    will set the password of the \code{admin} user to \code{s3cret}:

 
     \code{"; UPDATE users SET password='\$1\$OWgsBb90\$Lkko6aZwmp9XOVrFI09Ab0' WHERE \\username='admin' AND 'a' = "a}
 
 
     \code{"; UPDATE users SET password='\$1\$OWgsBb90\$Lkko6aZwmp9XOVrFI09Ab0' WHERE \\username='admin' AND 'a' = "a}
 


@@ -67,7 +71,7 @@ entered.
 
 \begin{result}
     The application allows the user to use any password (except ones that contain
 
 \begin{result}
     The application allows the user to use any password (except ones that contain

-    SQL code).
+       \SQL{} code).

 \end{result}
 
 \item
 \end{result}
 
 \item


@@ -89,7 +93,7 @@ authentication mechanism.
     expire, however. It would be even better to require that a token be used
     withing a day (or so) after creation.
 
     expire, however. It would be even better to require that a token be used
     withing a day (or so) after creation.
 

-    The security of the SMTP connection is at the discretion of the web server.
+       The security of the \SMTP{} connection is at the discretion of the web server.

     Often these connections are not very secure, but this worry is beyond the
     scope of this audit.
 \end{result}
     Often these connections are not very secure, but this worry is beyond the
     scope of this audit.
 \end{result}


@@ -126,11 +130,11 @@ salt, and there is sufficient work factor to defeat brute
 force and password hash recovery attacks.
 
 \begin{result}
 force and password hash recovery attacks.
 
 \begin{result}

-    Password are stored in database using the PHP function \texttt{crypt}. Internally, this
+       Password are stored in database using the \PHP{} function \code{crypt}. Internally, this

     function uses salted MD5. This is way too reverse with brute-force attacks using dictionary files.
 
     function uses salted MD5. This is way too reverse with brute-force attacks using dictionary files.
 

-    Instead it would be better to use the \texttt{argon2} password hashing algorithm
-    or the PHP \texttt{password\_hash} function (which currently uses BCRYPT).
+    Instead it would be better to use the \code{argon2} password hashing algorithm
+       or the \PHP{} \code{password\_hash} function (which currently uses BCRYPT).

 \end{result}
 
 \setcounter{enumi}{15}
 \end{result}
 
 \setcounter{enumi}{15}


@@ -143,9 +147,9 @@ user to enter credentials are done so using an encrypted
 link.
 
 \begin{result}
 link.
 
 \begin{result}

-    The app allows admin users to log in over HTTP. This is insecure, as it allows
+       The app allows admin users to log in over \HTTP{}. This is insecure, as it allows

     eavesdroppers to intercept password.
     eavesdroppers to intercept password.

-    The app should force HTTPS for at least the login form, the \texttt{admin\_controller} and
+       The app should force \HTTPS{} for at least the login form, the \code{admin\_controller} and

     for the installation script (because the users posts secrets like the database
     password to this page).
 \end{result}
     for the installation script (because the users posts secrets like the database
     password to this page).
 \end{result}


@@ -169,7 +173,7 @@ Verify that information enumeration is not possible via
 login, password reset, or forgot account functionality.
 
 \begin{result}
 login, password reset, or forgot account functionality.
 
 \begin{result}

-    All these forms are vulnerable to SQL injection attacks. So any information
+    All these forms are vulnerable to \SQL{} injection attacks. So any information

     can leak any information from the database.
 \end{result}
 
     can leak any information from the database.
 \end{result}
 


@@ -181,7 +185,7 @@ application (such as “admin/password”).
 
 \begin{result}
     No secrets are initialized by predefined values. The admin user will have
 
 \begin{result}
     No secrets are initialized by predefined values. The admin user will have

-    username \texttt{admin} by default. This is no secret and therefore not
+    username \code{admin} by default. This is no secret and therefore not

     considered unsafe.
 \end{result}
 
     considered unsafe.
 \end{result}
 


@@ -199,7 +203,8 @@ attacks.
         \item Email validation, to harden brute force email address discovery
         \item Installation database check, to prevent guessing attacks for the database password
         \item Login, to prevent login guessing
         \item Email validation, to harden brute force email address discovery
         \item Installation database check, to prevent guessing attacks for the database password
         \item Login, to prevent login guessing

-        \item And comment submission, to prevent spam, phishing et cetera (by using CAPTCHA).
+        \item And comment submission, to prevent spam, phishing et cetera (by
+        using some CAPTCHA software).

     \end{itemize}
 \end{result}
 
     \end{itemize}
 \end{result}
 


@@ -210,24 +215,24 @@ services external to the application are encrypted and
 stored in a protected location.
 
 \begin{result}
 stored in a protected location.
 
 \begin{result}

-    The database credentials are hardcoded in \texttt{config.php}. While it
+    The database credentials are hardcoded in \code{config.php}. While it

     would be better to pass secrets as environment variables, this is not
     really bad practice.
 
     However, the installation instructions state the following:
     \begin{verbatim}
     would be better to pass secrets as environment variables, this is not
     really bad practice.
 
     However, the installation instructions state the following:
     \begin{verbatim}

-Change the file permissions to allow all users write access to the folder
-you extracted testcms to.
+Change the file permissions to allow all users write access to the
+folder you extracted testcms to.

     \end{verbatim}
     This implies making the configuration file readable for all users on the
     system. This information should not be accessible for any user other than
     \end{verbatim}
     This implies making the configuration file readable for all users on the
     system. This information should not be accessible for any user other than

-    running the PHP script.
+       running the \PHP{} script.

 \end{result}
 
 \item
 \pass{}
 Verify that forgotten password and other recovery paths
 \end{result}
 
 \item
 \pass{}
 Verify that forgotten password and other recovery paths

-use a TOTP or other soft token, mobile push, or other
+use a \TOTP{} or other soft token, mobile push, or other

 offline recovery mechanism. Use of a random value in an
 e-mail or SMS should be a last resort and is known weak.
 
 offline recovery mechanism. Use of a random value in an
 e-mail or SMS should be a last resort and is known weak.
 


@@ -236,20 +241,28 @@ e-mail or SMS should be a last resort and is known weak.
     e-mail, which is considered weak (but not unsafe).
 \end{result}
 
     e-mail, which is considered weak (but not unsafe).
 \end{result}
 

-\notapplicable{\item
+\item
+\fail{}

 Verify that account lockout is divided into soft and hard
 lock status, and these are not mutually exclusive. If an
 account is temporarily soft locked out due to a brute force
 attack, this should not reset the hard lock status.
 
 Verify that account lockout is divided into soft and hard
 lock status, and these are not mutually exclusive. If an
 account is temporarily soft locked out due to a brute force
 attack, this should not reset the hard lock status.
 

-%     The application has not implemented any lockout mechanisms.
-}
+\begin{result}
+    The application has not implemented any lockout mechanisms.
+\end{result}

 
 

-\notapplicable{\item
+\item
+\pass{}

 Verify that if shared knowledge based questions (also
 Verify that if shared knowledge based questions (also

-known as "secret questions") are required, the questions
+known as ``secret questions'') are required, the questions

 do not violate privacy laws and are sufficiently strong to
 do not violate privacy laws and are sufficiently strong to

-protect accounts from malicious recovery.}
+protect accounts from malicious recovery.
+
+\begin{result}
+    The application uses no shared knowledge based questions, and thus not
+    violate any privacy laws.
+\end{result}

 
 \item
 \fail{}
 
 \item
 \fail{}


@@ -261,12 +274,15 @@ use of a configurable number of previous passwords.
     require variation in the use of different passwords.
 \end{result}
 
     require variation in the use of different passwords.
 \end{result}
 

-\notapplicable{\item
+\item
+\pass{}

 Verify that risk based re-authentication, two factor or
 Verify that risk based re-authentication, two factor or

-transaction signing is in place for high value transactions.}
+transaction signing is in place for high value transactions.

 
 

-% There are no (really) risk based action or which re-authentication would be
-% fit
+\begin{result}
+    There are no (really) risk based action or which re-authentication would be
+    fit.
+\end{result}

 
 \item
 \fail{}
 
 \item
 \fail{}


@@ -275,39 +291,48 @@ commonly chosen passwords and weak passphrases.
 
 \begin{result}
     No password strengthening measures are implemented. The app should
 
 \begin{result}
     No password strengthening measures are implemented. The app should

-    use some password strength estimator like \texttt{zxcvbn}.
+    use some password strength estimator like \texttt{zxcvbn}\footnote{\url{https://github.com/dropbox/zxcvbn}}.

 \end{result}
 
 \end{result}
 

+\notapplicable{

 \item
 \item

-\fail{}
+% \fail{}

 Verify that all authentication challenges, whether
 successful or failed, should respond in the same average
 response time.
 
 Verify that all authentication challenges, whether
 successful or failed, should respond in the same average
 response time.
 

-\begin{result}
-    String comparisation for checking password hases and password reset tokens
-    are not in constant time.
-\end{result}
+% \begin{result}
+%     String comparisation for checking password hashes and password reset
+%     tokens are not in constant time.
+% \end{result}
+}

 
 

+\notapplicable{

 \item
 \item

-\fail{}
-Verify that secrets, API keys, and passwords are not
+% \fail{}
+Verify that secrets, \API{} keys, and passwords are not

 included in the source code, or online source code
 repositories.
 
 included in the source code, or online source code
 repositories.
 

-\begin{result}
-    The database credentials are hard coded in \texttt{config.php}. These
-    credentials should ideally be passed using environment variables.
-\end{result}
+% \begin{result}
+%     The database credentials are hard coded in \code{config.php}. These
+%     credentials should ideally be passed using environment variables.
+% \end{result}
+}

 
 \setcounter{enumi}{30}
 
 
 \setcounter{enumi}{30}
 

-\notapplicable{\item
+\item
+\fail{}

 Verify that if an application allows users to authenticate,
 they can authenticate using two-factor authentication or
 other strong authentication, or any similar scheme that
 provides protection against username + password
 Verify that if an application allows users to authenticate,
 they can authenticate using two-factor authentication or
 other strong authentication, or any similar scheme that
 provides protection against username + password

-disclosure.}
+disclosure.
+
+\begin{result}
+    No surch features are implemented.
+\end{result}

 
 \item
 \fail{}
 
 \item
 \fail{}