Merge branch 'reflection_development' into 'master'
authorWouter Kuhnen <w.kuhnen@student.science.ru.nl>
Fri, 25 Nov 2016 09:58:23 +0000 (10:58 +0100)
committerWouter Kuhnen <w.kuhnen@student.science.ru.nl>
Fri, 25 Nov 2016 09:58:23 +0000 (10:58 +0100)
add centralization comments

See merge request !7

TODO.txt
report/fortify.tex
report/reflection.bottlenecks.tex [new file with mode: 0644]
report/reflection.code_and_auditing.tex
report/reflection.tex
report/reflection.tools.tex [new file with mode: 0644]
report/v2_input.tex [deleted file]

index 5648089..743a4a0 100644 (file)
--- a/TODO.txt
+++ b/TODO.txt
@@ -27,10 +27,10 @@ Reflect on the whole process, including
    and possibly also the TestCMS code.
 
 For example, questions to consider are:
-   - Kelley:  How good (useful, clear, ...) is the ASVS? How could it be improved?
-   - Charlie: How useful were code analysis tools? How could they be improved? How did you experience the rates and amounts of false and true positives? How might that be improved?
-   - Wouter:  What were the bottlenecks in doing the security review in your experience?
-   - Mart:    Maybe in the points above you can distinguish different (types of) security flaws or verification requirements. E.g., are some (categories of) verification requirements easier to check than others?
-   - Daan:    If you would have to do something like this again, what would you do differently? Eg. about organising things within the group: i.e., in retrospect, what do you think the best approach is to organise and divide the work in a team? (Dividing the verification requirements over the team members? Or by dividing the code? Or letting everyone look at everything, because different people will spot different things? Or work in pairs where one person confirms the findings of the other? ...)
-   - Kelley:  About the TestCMS code: are there important aspects that could (or should) be changed to improve security? Or aspects that could be changed to facilitate doing a security review?
-   - Wouter:  This last question might be generalised, to (web) applications in general, rather than this specific example of the TestCMS: ie. if you were to develop an application that will need to be subjected to a security review, would you do anything differently, given your experience in doing this, and if so, what and why?
+   - [x] Kelley:  How good (useful, clear, ...) is the ASVS? How could it be improved?
+   - [/] Charlie: How useful were code analysis tools? How could they be improved? How did you experience the rates and amounts of false and true positives? How might that be improved?
+   - [x] Wouter:  What were the bottlenecks in doing the security review in your experience?
+   - [ ] Mart:    Maybe in the points above you can distinguish different (types of) security flaws or verification requirements. E.g., are some (categories of) verification requirements easier to check than others?
+   - [x] Daan:    If you would have to do something like this again, what would you do differently? Eg. about organising things within the group: i.e., in retrospect, what do you think the best approach is to organise and divide the work in a team? (Dividing the verification requirements over the team members? Or by dividing the code? Or letting everyone look at everything, because different people will spot different things? Or work in pairs where one person confirms the findings of the other? ...)
+   - [ ] Daan:    About the TestCMS code: are there important aspects that could (or should) be changed to improve security? Or aspects that could be changed to facilitate doing a security review?
+   - [/] Wouter:  This last question might be generalised, to (web) applications in general, rather than this specific example of the TestCMS: ie. if you were to develop an application that will need to be subjected to a security review, would you do anything differently, given your experience in doing this, and if so, what and why?
index 28cbb27..898597b 100644 (file)
@@ -7,7 +7,7 @@ Fortify's results can be summarized to the following:
 \begin{enumerate}[label=(\Alph*)]
   \item 50 cases of \XSS{} vurnerabilities, all labeled \textbf{critical}, because none of the \CMS{}'s forms include nonces / protection against \XSS{} is indeed missing.
   \item \textbf{Password management}. In a user password reset form in \code{reset.php}, if the resetting fails, the password the user just entered reappears in the password field. This is not a database-retrieved password, and hence not actually as \textbf{critical} as Fortify labels it, but of course bad practice nonetheless.
-  \item In the \textbf{privact violation} category, Fortify found errors and warnings printed back to the browser, and labelled it \textbf{critical}. However, this happens in the installer script, which we have decided to treat separately, as explained earlier.
+  \item In the \textbf{privacy violation} category, Fortify found errors and warnings printed back to the browser, and labelled it \textbf{critical}. However, this happens in the installer script, which we have decided to treat separately, as explained earlier.
   \item \textbf{\SQL{} injection} attacks are possible on the installer script, labelled \textbf{critical}. Yet again: the installer script.
   \item \textbf{Cookie security}: the \code{HttpOnly} header is not set, labelled \textbf{high}.
   \item \textbf{Privacy violation}: \HTML{} forms don't disable autocompletion. Labelled \textbf{high}. However, autocompletion of \HTML{} forms by means of the \code{autocompletion="none"} attribute notoriously doesn't really work. The larger problem is that the post/redirect/get pattern is not followed, as stated above at our analysis of OWASP requirement (9.1).
@@ -42,53 +42,52 @@ For this reason, Fortify was nowhere near able to identifying all the problems w
 \begin{table}[th!]
 \centering
 %\renewcommand{\arraystretch}{1}
-\begin{tabular}{@{}lllllllllll@{}}
+\begin{tabular}{@{}llllllllll@{}}
 \toprule
 \# &
 \textbf{V2} &
 \textbf{V3} &
 \textbf{V4} &
-\textbf{V5} &
+\textbf{V5 (6)} &
 \textbf{V7} &
 \textbf{V8} &
 \textbf{V9} &
-\textbf{V12} &
-\textbf{V17} \\
+\textbf{V11} \\
 \midrule
-%      V2     V3     V4     V5     V7        V8     V9     V12    V17
- 1 &   \X    &   \p &   \p    &   \p &         &   \X & \F{B}\X &   \X &   \TODO \\
- 2 & \F{B}\p &   \p &         &      &   \p    &   \p &       &   \p &   \TODO \\
- 3 &         &   \X &         &   \X &         &   \X &    \p &      &   \TODO \\
- 4 &   \p    &      &   \p    &      &         &   \X &    \X &   \X &   \TODO \\
- 5 &         &   \p &   \p    &   \p &         &   \p &    \p &   \p &   \TODO \\
- 6 &   \X    &   \p &         &      &         &   \p &       &   \X &   \TODO \\
- 7 &   \p    &   \X &         &      &   \TODO &   \p &    \p &   \X &   \TODO \\
- 8 &   \p    &      &   \p    &      &         &      &       &   \X &         \\
- 9 &   \X    &   \p &   \X    &      &         &      &    \p &      &   \TODO \\
-10 &         &   \X &   \p    &   \X &         &   \X &    \p &      &   \TODO \\
-11 &         &   \p &         &   \p &         &      &    \p &      &   \TODO \\
-12 &   \X    &   \X &   \X    &   \p &         &      &       &      &         \\
-13 &   \X    &   \X & \F{A}\X &   \p &         &   \X &       &      &         \\
-14 &         &      &   \X    &   \p &         &      &       &      &         \\
-15 &         &      &   \X    &   \X &         &      &       &      &         \\
-16 &   \X    &      &   \X    &   \p &         &      &       &      &         \\
-17 &   \p    &      &         &   \p &         &      &       &      &         \\
-18 &   \X    &      &         &   \X &         &      &       &      &         \\
-19 &   \p    &      &         &   \X &         &      &       &      &         \\
-20 &   \X    &      &         &   \p &         &      &       &      &         \\
-21 &   \X    &      &         &   \p &         &      &       &      &         \\
-22 &   \p    &      &         &   \X &         &      &       &      &         \\
-23 &         &      &         &   \X &         &      &       &      &         \\
-24 &         &      &         &   \p &         &      &       &      &         \\
-25 &   \X    &      &         &   \p &         &      &       &      &         \\
-26 &         &      &         &   \p &         &      &       &      &         \\
-27 &   \X    &      &         &      &         &      &       &      &         \\
-28 &   \X    &      &         &      &         &      &       &      &         \\
-29 &   \X    &      &         &      &         &      &       &      &         \\
-30 &         &      &         &      &         &      &       &      &         \\
-31 &         &      &         &      &         &      &       &      &         \\
-32 &   \X    &      &         &      &         &      &       &      &         \\
-33 &   \p    &      &         &      &         &      &       &      &         \\
+%       V2       V3     V4        V5     V7        V8      V9      V11
+ 1 &   \X    &   \p &   \p    &   \p &         &   \X & \F{B}\X &   \X  \\
+ 2 & \F{B}\p &   \p &         &      &   \p    &   \p &         &   \p  \\
+ 3 &         &   \X &         &   \X &         &   \X &    \p   &       \\
+ 4 &   \p    &      &   \p    &      &         &   \X &    \X   &   \X  \\
+ 5 &         &   \p &   \p    &   \p &         &   \p &    \p   &   \p  \\
+ 6 &   \X    &   \p &         &      &   \X    &   \p &         &   \X  \\
+ 7 &   \p    &   \X &         &      &   \p    &   \p &    \p   &   \X  \\
+ 8 &   \p    &      &   \p    &      &         &      &         &   \X  \\
+ 9 &   \X    &   \p &   \X    &      &   \p    &      &    \p   &       \\
+10 &         &   \X &   \p    &   \X &         &   \X &    \p   &       \\
+11 &         &   \p &         &   \p &         &      &    \p   &       \\
+12 &   \X    &   \X &   \X    &   \p &   \X    &      &         &       \\
+13 &   \X    &   \X & \F{A}\X &   \p &   \X    &   \X &         &       \\
+14 &         &      &   \X    &   \p &   \p    &      &         &       \\
+15 &         &      &   \X    &   \X &         &      &         &       \\
+16 &   \X    &      &   \X    &   \p &         &      &         &       \\
+17 &   \p    &      &         &   \p &         &      &         &       \\
+18 &   \X    &      &         &   \X &         &      &         &       \\
+19 &   \p    &      &         &   \X &         &      &         &       \\
+20 &   \X    &      &         &   \p &         &      &         &       \\
+21 &   \X    &      &         &   \p &         &      &         &       \\
+22 &   \p    &      &         &   \X &         &      &         &       \\
+23 &         &      &         &   \X &         &      &         &       \\
+24 &         &      &         &   \p &         &      &         &       \\
+25 &   \X    &      &         &   \p &         &      &         &       \\
+26 &         &      &         &   \p &         &      &         &       \\
+27 &   \X    &      &         &      &         &      &         &       \\
+28 &   \X    &      &         &      &         &      &         &       \\
+29 &   \X    &      &         &      &         &      &         &       \\
+30 &         &      &         &      &         &      &         &       \\
+31 &         &      &         &      &         &      &         &       \\
+32 &   \X    &      &         &      &         &      &         &       \\
+33 &   \p    &      &         &      &         &      &         &       \\
 \bottomrule
 \end{tabular}
 \caption{Summary of our results. Fortify's findings are outlined and labelled, see our analysis above.}
diff --git a/report/reflection.bottlenecks.tex b/report/reflection.bottlenecks.tex
new file mode 100644 (file)
index 0000000..42ced32
--- /dev/null
@@ -0,0 +1,38 @@
+ % Wouter:  What were the bottlenecks in doing the security review in your
+ % experience?
+
+% Only working on part of the code: 'big picture' may be missing
+When we divided the work that had to be done for the security audit we
+settled on giving each each member two categories to check. Then later we all
+verified each other's findings.
+
+This approach meant that everyone got very familiar with a certain
+functionality of the web application. But it also created a bottleneck in
+the sense that it is easy to lose overview of the application as the sum of
+its parts. We feel this bottleneck is an acceptable consequence of the way we
+divided the workload.
+
+% Finding the code which handles a requirement (failing is easier than pass)
+Another bottleneck we ran into when failing a requirement was not trivial, was
+finding the actual code which handles a requirement. When assessing if a certain
+requirement passes or fails the OWASP guidelines, one has to find the code which
+implements the requirement. For many of the requirements this comes down to
+finding a counterexample. In our experience there are not often very hard to
+find for this specific web application. However for some requirements
+counterexamples will be harder to find. For these we needed to find all the code
+which touches upon this requirement and then verify it.
+
+To summarize the previous paragraph: it is almost always easier to fail a
+requirement and be sure of it, then to pass it. Because passing means that the
+requirement is OK in \emph{all} cases. While failing needs only to mean that an
+requirement fails in \emph{one} case.
+
+% Verifying each other's results
+The final bottleneck we like to note concerns the verifying of each other's
+results. When verifying the results of your group members (collegues) it may be
+difficult to fully double-check their work. This touches upon the previous
+point, for when a counterexample is giving it is trivial to see the result is
+correct. However when a member of the group passes a requirement it will take
+more time to verify this is the correct verdict. In the end we are confident of
+our results, but this may be a bottleneck which can be addressed in the initial
+organisation.
index e9d0f20..9b3f7d1 100644 (file)
@@ -34,16 +34,20 @@ some components, like input escaping, are just not present.
 \subsection{On our auditing process}
 \input{reflection.auditing_process.tex}
 
-(TODO: Wouter)
+\subsection{On the bottlenecks}
+\input{reflection.bottlenecks.tex}
 
 \subsection{On the ASVS checklist}
 \input{reflection.asvs.tex}
 
 \subsection{On HP Fortify / automated code analysis tools}
-(TODO: Charlie)
+\input{reflection.tools.tex}
 
 \subsection{?}
 (TODO: Mart)
 
 \subsection{On the code \& streamlining subsequent security audits}
 \input{reflection.code_and_auditing.tex}
+
+\subsection{On the general development of secure software}
+\input{reflection.secure_development.tex}
diff --git a/report/reflection.tools.tex b/report/reflection.tools.tex
new file mode 100644 (file)
index 0000000..c03d227
--- /dev/null
@@ -0,0 +1,23 @@
+\r
+% How useful were code analysis tools?\r
+The usefulness of the Fortify Static Code Analysis tool turned out to be very limited.\r
+Since we had most verdicts ready before a license was provided we couldn't use\r
+the tool as an initial guide trough the code. This forced us to manually check\r
+the application source which took quite some time. After the tool became available we\r
+didn't get any new insights regarding potential security risks, just more examples\r
+of problems we already detected.\r
+\r
+% How could they be improved? (niet echt een antwoord maar we hebben de tool ook niet echt gebruikt?)\r
+In our opinion the tool could have proved very useful in pointing out certain security\r
+flaws in the initial stage of this project since we spent a lot of time scanning the\r
+application code-base. Since Fortify located relatively low-level problems we could\r
+have used these to locate potential hot-spots. \r
+Saving us from going trough every source file and trying to determine if they are part of the\r
+applications external access points. In order to improve upon the tool we suggest a larger\r
+focus on determining which parts of a application need to be secure and less on pointing\r
+out actual security flaws.\r
+\r
+% How did you experience the rates and amounts of false and true positives?\r
+TODO: feedback per groepslid, ik heb geen idee hoe iedereen dit ervaren heeft.\r
+\r
+% How might that be improved?\r
diff --git a/report/v2_input.tex b/report/v2_input.tex
deleted file mode 100644 (file)
index e69de29..0000000