X-Git-Url: https://git.martlubbers.net/?a=blobdiff_plain;ds=sidebyside;f=report%2Fv2_authentication.tex;h=69d1d00693b0dd73d2702de8a5b046165e7dae8c;hb=b52c24b2c26384817d042703a46eee317a85535e;hp=08c35bef8010141942fe961cc7233a4cd288e720;hpb=06d7ae862f750a89968a0c7c2104b133e801d3a5;p=ssproject1617.git diff --git a/report/v2_authentication.tex b/report/v2_authentication.tex index 08c35be..69d1d00 100644 --- a/report/v2_authentication.tex +++ b/report/v2_authentication.tex @@ -15,15 +15,19 @@ public (Principle of complete mediation). \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} -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} @@ -48,10 +52,10 @@ attackers cannot log in. \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 - 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} @@ -67,7 +71,7 @@ entered. \begin{result} The application allows the user to use any password (except ones that contain - SQL code). + \SQL{} code). \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. - 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} @@ -126,11 +130,11 @@ salt, and there is sufficient work factor to defeat brute force and password hash recovery attacks. \begin{result} - Password are stored in database using the PHP function \texttt{crypt}. Internally, this - function uses salted MD5. This is way easy too reverse with brute-force attacks using dictionary files. + 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. - 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} @@ -143,9 +147,9 @@ user to enter credentials are done so using an encrypted 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. - 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} @@ -169,7 +173,7 @@ Verify that information enumeration is not possible via 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} @@ -181,7 +185,7 @@ application (such as “admin/password”). \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} @@ -211,24 +215,24 @@ services external to the application are encrypted and 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} -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 - the one running the PHP script. + running the \PHP{} script. \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. @@ -248,7 +252,7 @@ attack, this should not reset the hard lock status. \notapplicable{\item 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 protect accounts from malicious recovery.} @@ -292,12 +296,12 @@ response time. \item \fail{} -Verify that secrets, API keys, and passwords are not +Verify that secrets, \API{} keys, and passwords are not included in the source code, or online source code repositories. \begin{result} - The database credentials are hard coded in \texttt{config.php}. These + The database credentials are hard coded in \code{config.php}. These credentials should ideally be passed using environment variables. \end{result}