Merge branch 'master' of gitlab.science.ru.nl:mlubbers/ssproject1617 into dsprenkels...
authorDaan Sprenkels <dsprenkels@gmail.com>
Mon, 7 Nov 2016 14:35:33 +0000 (15:35 +0100)
committerDaan Sprenkels <dsprenkels@gmail.com>
Mon, 7 Nov 2016 14:35:33 +0000 (15:35 +0100)
OWASP_Application_Security_Verification_Standard_3.0.1.pdf [new file with mode: 0644]
report/.chktexrc [new file with mode: 0644]
report/Makefile
report/preamble.tex
report/report.tex
report/v4_access.tex [new file with mode: 0644]
report/v5_input.tex

diff --git a/OWASP_Application_Security_Verification_Standard_3.0.1.pdf b/OWASP_Application_Security_Verification_Standard_3.0.1.pdf
new file mode 100644 (file)
index 0000000..fb94159
Binary files /dev/null and b/OWASP_Application_Security_Verification_Standard_3.0.1.pdf differ
diff --git a/report/.chktexrc b/report/.chktexrc
new file mode 100644 (file)
index 0000000..b16a8df
--- /dev/null
@@ -0,0 +1 @@
+WipeArg { \code:{} \url:{} }
index 42c9c52..d82ce26 100644 (file)
@@ -5,7 +5,8 @@ LATEXFLAGS:=-file-line-error -halt-on-error -no-shell-escape
 
 TEXS:=$(wildcard *.tex)
 
-.PHONY: all $(DOC).fmt
+.PHONY: all
+.SECONDARY: $(DOC).fmt
 
 all: $(DOC).pdf
 
index 8bc003b..692cea3 100644 (file)
@@ -1,16 +1,53 @@
 \documentclass[a4paper,titlepage]{article}
 
+\usepackage{CJKutf8}
 \usepackage{rutitlepage}
 \usepackage{geometry}
 \usepackage{hyperref}
 \usepackage{enumitem}
+\usepackage{pifont}
 \usepackage[dvipsnames]{xcolor}
+\usepackage{mdframed}
 
 \hypersetup{hidelinks, pdftitle={OWASP ASVS Souce Code Review Project}}
 
 % Als een criterium niet applicable is (we doen alleen 1 en 2)
 \newcommand{\notapplicable}[1]{{\color{Gray} #1}}
 
+% Layout voor resultaat van requirement check.
+% (Anders loopt de tekst vd requirement direct over in het resultaat, wat niet duidelijk leest.)
+\newmdenv[
+  topline=false,
+  bottomline=false,
+  rightline=false,
+  linewidth=1.5pt,
+  innerrightmargin=0pt,
+  innertopmargin=2pt,
+  innerbottommargin=2pt,
+  skipabove=\topsep,
+  skipbelow=\topsep
+]{result}
+
+% Tools afkortingen
+\newcommand{\PHP}{\textsc{PHP}}
+\newcommand{\SQL}{\textsc{SQL}}
+\newcommand{\LDAP}{\textsc{LDAP}}
+\newcommand{\XML}{\textsc{XML}}
+\newcommand{\HTML}{\textsc{HTML}}
+\newcommand{\JSON}{\textsc{JSON}}
+\newcommand{\JQuery}{\textsc{JQuery}}
+
+% Reference naar de source
+\newcommand{\srcref}[2]{{\small\texttt{#1}} (line (s) #2)}
+
+% Pass en fail
+\newcommand{\pass}{{\large\ding{51}}}
+\newcommand{\fail}{{\large\ding{55}}}
+\newcommand{\TODO}{\ding{111} \textbf{\itshape TODO}}
+
+% span code fragments
+\newcommand{\code}[1]{\texttt{#1}}
+
 \renewcommand\thesubsection{V\arabic{subsection}}
 
 \author{%
index d44b94c..51a35d3 100644 (file)
@@ -5,18 +5,25 @@
 \input{organization.tex}
 
 \section{Verdict}
+
+\subsection*{General comments}
+
+\begin{itemize}
+  \item \TODO{} (Kelley) Server instellingen.
+  \item \TODO{} (Kelley) \code{installer.php} and server code execution.
+\end{itemize}
+
 \addtocounter{subsection}{1}
 \subsection{Authentication}
 
 \subsection{Session Management}
 
 \subsection{Access Control}
+\input{v4_access.tex}
 
-\subsection{Input Validation}
+\subsection{Input Validation \& Output Encoding/Escaping}
 \input{v5_input.tex}
 
-\subsection{Output Encoding/Escaping}
-
 \subsection{Cryptography at rest}
 
 \subsection{Error Handling \& logging}
diff --git a/report/v4_access.tex b/report/v4_access.tex
new file mode 100644 (file)
index 0000000..506eea7
--- /dev/null
@@ -0,0 +1,165 @@
+
+\noindent
+The CMS has the following access control mechanisms:
+
+\begin{itemize}
+  \item A login mechanism, where logged in users are allowed to access the backend, and anonymous users are not.
+  \item For logged in users, a role system with three specified roles: admin, editor and regular user.
+  \item Different users (cap.~diff?)
+\end{itemize}
+
+\noindent
+Some typical objects of access control, in this case, are:
+
+\begin{itemize}
+  \item Database records
+  \item Editing capabilities
+  \item Role privileges
+  \item Folders, files, info over these data
+\end{itemize}
+
+Our check reveales that the access control mechanisms are basically only a stub, and haven't been developed to their usually implied meaning, thus flattening the access control to the single ascept of being logged in or not. Hence, the main remaining security consideration deal with whether this login mechanism protects `backend' objects from anonymous users.
+
+These are the results of our check:
+
+\begin{enumerate}[label={4.\arabic*}]
+
+% Access controls:
+%   - principle of least privilege?
+%   - inter-role/context safety
+%   - fail safely?
+%   - presentation -- server side
+%   - cannot be manipulated
+%   - onion layers
+%   - safe/working auth in place
+
+\item
+\fail{}
+Verify that the principle of least privilege exists - users
+should only be able to access functions, data files, URLs,
+controllers, services, and other resources, for which they
+possess specific authorization. This implies protection
+against spoofing and elevation of privilege.
+
+\begin{result}
+The login system that is in place seems to work, and at least theoretically only allows registered users to access the backend.
+
+There is a role system, but it seems only to be a stub, for the role of a user has no effect whatsoever on their capabilities.
+\end{result}
+
+\addtocounter{enumi}{2}
+\item
+\fail{}
+Verify that access to sensitive records is protected, such
+that only authorized objects or data is accessible to each
+user (for example, protect against users tampering with a
+parameter to see or alter another user's account).
+
+\begin{result}
+There is no different access context for distinct users. In particular, they are allowed to access and edit each others' account info, including password. Taken one way, this could be said to be by design and thus OK. But in any reasonable design concept allowing for distinct user accounts, this is clearly not the desired setup.
+\end{result}
+
+\item
+\fail{}
+Verify that directory browsing is disabled unless
+deliberately desired. Additionally, applications should not
+allow discovery or disclosure of file or directory metadata,
+such as \code{Thumbs.db}, \code{.DS\_Store}, \code{.git} or \code{.svn} folders.
+
+\begin{result}
+\begin{itemize}[leftmargin=*]
+  \item \code{.gitignore} accessible, as well as any other dot-preceded file (except \code{.htaccess} itself by default Apache rules), as well as files such as \code{Thumbs.db} and \code{.DS\_Store}.
+  \item Directory contents were listed in my simple setup. A global apache setting may disable by default, but the \code{.htaccess} file doesn't explicitly disable (with \code{Options -Indexes}), so that the CMS's codebase basically enables the listing by default.
+\end{itemize}
+\end{result}
+
+\addtocounter{enumi}{2}
+\item
+\pass{}
+Verify that access controls fail securely.
+
+\begin{result}
+The only remaining access control mechanism is that of logging in. It fails securely, in the sense that no info about the correctness of the username/password pair are leaked (response time analysis not included); the single error message reads: ``Incorrect details.''
+\end{result}
+
+\item
+\fail{}
+Verify that the same access control rules implied by the
+presentation layer are enforced on the server side.
+
+\begin{result}
+Fail, because the role and distinct user systems are stubs.
+\end{result}
+
+\item
+\pass{}
+Verify that all user and data attributes and policy
+information used by access controls cannot be
+manipulated by end users unless specifically authorized.
+
+\begin{result}
+This item is the main remaining security concern. I haven't found any obvious fail in the login system, but given the architecture and security status of the whole CMS, I'm not very sure of it.
+\end{result}
+
+\notapplicable{
+\item
+Verify that there is a centralized mechanism (including
+libraries that call external authorization services) for
+protecting access to each type of protected resource.
+}
+
+\item
+\fail{}
+Verify that all access control decisions can be logged and
+all failed decisions are logged.
+
+\begin{result}
+No such decision logging present. There is only a minor amount of logging, and this is related to not finding content (pages and articles).
+\end{result}
+
+\item
+\fail{}
+Verify that the application or framework uses strong
+random anti-CSRF tokens or has another transaction
+protection mechanism.
+
+\begin{result}
+There is no transation protection mechanism at all.
+\end{result}
+
+\item
+\fail{}
+Verify the system can protect against aggregate or
+continuous access of secured functions, resources, or
+data. For example, consider the use of a resource
+governor to limit the number of edits per hour or to
+prevent the entire database from being scraped by an
+individual user.
+
+\begin{result}
+No such prevention present.
+\end{result}
+
+\item
+\fail{}
+Verify the application has additional authorization (such
+as step up or adaptive authentication) for lower value
+systems, and / or segregation of duties for high value
+applications to enforce anti-fraud controls as per the risk
+of application and past fraud.
+
+\begin{result}
+No such mechanisms.
+\end{result}
+
+\item
+\fail{}
+Verify that the application correctly enforces context-
+sensitive authorisation so as to not allow unauthorised
+manipulation by means of parameter tampering.
+
+\begin{result}
+No such mechanisms.
+\end{result}
+
+\end{enumerate}
index 618c2bf..7a01436 100644 (file)
-\begin{enumerate}[label=5.\arabic*]
-       \item Verify that the runtime environment is not susceptible to buffer
+\begin{enumerate}[label={5.\arabic*}]
+       \item\pass{} Verify that the runtime environment is not susceptible to buffer
                overflows, or that security controls prevent buffer overflows.
 
+               \begin{result}
+               As of \emph{OWASP}'s statement\footnote{\url{%
+                       https://www.owasp.org/index.php/Buffer_Overflows\#Platforms_Affected}}
+               \PHP{} is not surceptible to buffer overflows as long no external
+               programs or extensions are used which is not the case.
+               \end{result}
+
        % They skip 5.2
        \addtocounter{enumi}{1}
-       \item Verify that server side input validation failures result in 
+       \item\fail{} Verify that server side input validation failures result in 
                request rejection and are logged.
 
+               \begin{result}
+               \code{filter\_var} is used for input validation and while errors are
+               returned to the user, no logging taking place.
+               \end{result}
+
        % They skip 5.4
        \addtocounter{enumi}{1}
-       \item Verify that input validation routines are enforced on the server
-               side.
+       \item\pass{} Verify that input validation routines are enforced on the
+               server side.
+
+               \begin{result}
+               Errors are accumulated in an array which, when non-empty, will fail the
+               function and report the error.
+               \end{result}
 
-       \item\notapplicable{Verify that a single input validation control is used
+       \notapplicable{\item Verify that a single input validation control is used
                by the application for each type of data that is accepted.}
 
        % They skip 5.7-5.9
        \addtocounter{enumi}{3}
-       \item Verify that all SQL queries, HQL, OSQL, NOSQL and stored 
-               procedures, calling of stored procedures are protected by the 
-               use of prepared statements or query parameterization, and 
-               thus not susceptible to SQL injection.
+       \item\fail{} Verify that all \SQL{} queries, \code{HQL}, \code{OSQL},
+               \code{NOSQL} and stored procedures, calling of stored procedures are
+               protected by the use of prepared statements or query parameterization,
+               and thus not susceptible to \SQL{} injection.
 
-       \item Verify that the application is not susceptible to LDAP
+               \begin{result}
+               This is not the case. For example in \srcref{classes/users.php}{45}.
+               However, in some cases prepared statements are used, such as is
+               \srcref{classes/users.php}{145}.
+               \end{result}
+
+       \item\pass{} Verify that the application is not susceptible to LDAP
                Injection, or that security controls prevent LDAP Injection.
 
-       \item Verify that the application is not susceptible to OS Command
+               \begin{result}
+               \LDAP{} is not used, thus the application is not susceptible.
+               \end{result}
+
+       \item\pass{} Verify that the application is not susceptible to OS Command
                Injection, or that security controls prevent OS Command Injection.
+
+               \begin{result}
+               This requirement heavily depends on the configuration of the \PHP{}
+               interpreter and database, there are no system commands used but since
+               it is trivial to do an \SQL{} injection it might be possible to run
+               commands via the database. However, which a sufficiently secure \SQL{}
+               config this can not take place.
+               \end{result}
+
+       \item\pass{} Verify that the application is not susceptible to Remote File
+               Inclusion (RFI) or Local File Inclusion (LFI) when content is used that
+               is a path to a file.
+
+               \begin{result}
+               Some file inclusion might be possible in the themes. Also in password
+               recovery\\
+               (\srcref{classes/user.php}{115}) filepaths are calculated on the
+               hash of the password. All non standard filepaths, such as admin or
+               theme files, are generated using functions. CMS urls are parsed using a
+               standard system wide \code{parse} function.
+               \end{result}
+
+       \item\pass{} Verify that the application is not susceptible to common
+               \XML{} attacks, such as XPath query tampering, \XML{} External Entity
+               attacks, and \XML{} injection attacks. 
+
+               \begin{result}
+               No \XML{} or related techniques are used and thus the application is
+               not susceptible.
+               \end{result}
+
+       \item\fail{} Ensure that all string variables placed into \HTML{} or other
+               web client code is either properly contextually encoded manually, or
+               utilize templates that automatically encode contextually to ensure the
+               application is not susceptible to reflected, stored and DOM Cross-Site
+               Scripting (XSS) attacks.
+
+               \begin{result}
+               A lot of \HTML{} tags are allowed in the post screen, therefore an XSS
+               attack is trivial. Even the comment section uses no input validation
+               whatsoever.
+               \end{result}
+
+       \item\pass{} If the application framework allows automatic mass parameter
+               assignment (also called automatic variable binding) from the inbound
+               request to a model, verify that security sensitive fields such as
+               ``accountBalance'', ``role'' or ``password'' are protected from
+               malicious automatic binding.
+
+               \begin{result}
+               There is some automatic variable binding happening in the POST and GET
+               however, defaults are always given and there is no possibility of
+               accidentally binding extra variables. Also the variables are in an
+               array.
+               \end{result}
+
+       \item\pass{} Verify that the application has defenses against HTTP
+               parameter pollution attacks, particularly if the application framework
+               makes no distinction about the source of request parameters (GET, POST,
+               cookies, headers, environment, etc.)
+
+               \begin{result}
+               The system explicitly makes a difference with the different input
+               types. As said in the previous item, the function that does this
+               parameter parsing is system wide and uses defaults and filters unwanted
+               parameters.
+               \end{result}
+
+       \item\fail{} Verify that client side validation is used as a second line of
+               defense, in addition to server side validation.
+
+               \begin{result}
+               There is client side validation on comments in the email section. There
+               is no validation for the comments itself to check for malafide \HTML{}.
+               In the admin panel the email address is not validated.
+               \end{result}
+
+       \item\fail{} Verify that all input data is validated, not only \HTML{} form
+               fields but all sources of input such as REST calls, query parameters,
+               HTTP headers, cookies, batch files, RSS feeds, etc; using positive
+               validation (whitelisting), then lesser forms of validation such as
+               greylisting (eliminating known bad strings), or rejecting bad inputs
+               (blacklisting).
+
+               \begin{result}
+               REST calls are validated using whitelisting, query parameters are not,
+               headers are not, cookies not, batch files are non-existent and RSS feed
+               output is not filtered.
+               \end{result}
+
+       \item\pass{} Verify that structured data is strongly typed and validated
+               against a defined schema including allowed characters, length and
+               pattern (e.g.\  credit card numbers or telephone, or validating that two
+               related fields are reasonable, such as validating suburbs and zip or
+               post codes match). 
+
+               \begin{result}
+               Email addresses are validated against \PHP's stander functionality.
+               Note that the \PHP{} email validation is not perfect and some valid
+               email addresses are rejected (such as email addresses with non-ASCII
+               characters). The other requirements are not used.
+               \end{result}
+
+       \item\pass{} Verify that unstructured data is sanitized to enforce generic
+               safety measures such as allowed characters and length, and characters
+               potentially harmful in given context should be escaped (e.g.\ natural
+               names with Unicode or apostrophes, such as
+               \begin{CJK}{UTF8}{min}ねこ\end{CJK} or O'Hara)
+
+               \begin{result}
+               Emailaddresses with non-ASCII characters are rejected. Unicode
+               characters are displayed correctly.
+               \end{result}
+
+       \item\fail{} Make sure untrusted \HTML{} from WYSIWYG editors or similar are
+               properly sanitized with an \HTML{} sanitizer and handle it
+               appropriately according to the input validation task and encoding task. 
+
+               \begin{result}
+               This is not the case, any \HTML{} is allowed.
+               \end{result}
+
+       \item\fail{} For auto-escaping template technology, if UI escaping is disabled,
+               ensure that \HTML{} sanitization is enabled instead.
+
+               \begin{result}
+               See previous item.
+               \end{result}
+
+       \item\pass{} Verify that data transferred from one DOM context to another,
+               uses safe JavaScript methods, such as using \code{.innerText} and
+               \code{.val}.
+
+               \begin{result}
+               The \JQuery{} framework is used for this.
+               \end{result}
+
+       \item\pass{} Verify when parsing \JSON{} in browsers, that
+               \code{JSON.parse} is used to parse \JSON{} on the client. Do not use
+               \code{eval()} to parse \JSON{} on the client.
+
+               \begin{result}
+               There is no \JSON{} transfer outside the toolkits.
+               \end{result}
+
+       \item\pass{} Verify that authenticated data is cleared from client storage,
+               such as the browser DOM, after the session is terminated.
+
+               \begin{result}
+               No DOM storage is used.
+               \end{result}
+
 \end{enumerate}