https://wiki.owasp.org/api.php?action=feedcontributions&feedformat=atom&user=Carlos+PantelidesOWASP - User contributions [en]2026-05-14T11:07:36ZUser contributionsMediaWiki 1.27.2https://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=175398CRV2 CodeReviewAgile2014-05-21T14:49:14Z<p>Carlos Pantelides: </p>
<hr />
<div>==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you. Or better yet, integrate yourself to the Team.<br />
<br />
Code review must be done at least at the end of every user story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review. Nevertheless, as security review requieres some extra specialization, perhaps the best way is to integrate the security code reviewier with the Team.<br />
<br />
These are some simple steps on how to integrate code reviews in an agile project:<br />
<br />
* Configure your Continous Integration system to run static code analysis tools.<br />
* Train your team on this guide.<br />
* Train your team on OWASP TopTen.<br />
* Use this guide when doing Peer Review.<br />
* Determine the frequency of the review. A good place is at the end of every loop.<br />
* Keep in mind that besides the code, there is the test code.<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between. There are many flavors of agile, perhaps as many as practitioners. It is like an heterogeneous reference framework and you are free to use what you want.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed. First, when the review is done and then, the code itself.<br />
<br />
Agile Development(AD) is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity. Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the develop, test, code review cycle.<br />
<br />
It is a common practice to define short development cycles. At the end of each one, all the code must be production quality code. Its funtionality may be incomplete, but it must add some value. That affects the review process as it must be continuous.<br />
<br />
===Pair Programming===<br />
<br />
This technique is quite controversial, but when it is adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
In pair program there are 2 roles, the programmer (pig) and the supervisor (chicken). Basically, what the supervisor does is control the work of the programmer. In this process, a code review might be part of it and it can be easily integrated since the major responsibility of the supervisor is to regulate the programmer’s work.<br />
<br />
===Continuous Integration===<br />
<br />
It is a practice from eXtreme Programming (link to the book) based upon that team programming is not a divide and conquer problem but a divide, conquer and integrate problem and that integrate could be the hardest step. In order to mitigate the impact, the integration should be as frequently as there are commits to the repository. An automated task becomes aware of any change and triggers the build and test process. If there is any error, the committer get notified so he or she can fix it before it accumulates with other errors.<br />
<br />
===Peer Review===<br />
<br />
This one is enforced by the usage of tools like Jenkins that ask another user for a code review before commiting to the versioning system.<br />
<br />
===Testing===<br />
<br />
The role of testers... some people thinks that there is almost no place for the traditional tester role. Automated testing requires a tester to define the tests but not to execute them. This tester must be almost a programmer or work with a programmer.<br />
<br />
In AD, test is so fundamental, that the xDD pervades Agile, test first, test earlier.<br />
<br />
Agile projects tend to use a lot of automated testing, in order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
Some guide taken from (I can't remember but searching...)<br />
*Are there enough tests?<br />
*Is the code covered by the tests? Code coverage measure main value is to find unused code.<br />
*Are there the trivial tests? They are as needed as any other test.<br />
*Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.<br />
*Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.<br />
*Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?<br />
*Are the test automatic? If the tests requiere manual intervention, they will not be run.<br />
*Do the tests conform to F.I.R.S.T.?<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
===Refactoring===<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.<br />
<br />
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
==References and sources==<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
Extreme Programming Explained: embrace change - Kent Beck with Cynthia Andres<br />
<br />
http://groups.yahoo.com/group/foro-agiles/<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://martinfowler.com/bliki/TestCoverage.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/<br />
<br />
=I am not sure about keeping the following sections=<br />
<br />
<br />
===Clean Code and "Smells"===<br />
<br />
<br />
The agile community is very committed to code quality. A by product is the concept of Clean Code and its smells, its very handy to be familiar with this topic, ...<br />
<br />
==The Agile Ecosystem==<br />
<br />
bdd----------------------------------------------------------------------+<br />
| |<br />
tdd-------------------------------------------------------------+ |<br />
| | |<br />
+----+--> pair programming --------+--> peer review -----> continuous integration<br />
| ( collective ownership ) |<br />
+--> individual programming ---+<br />
<br />
<br />
===The XDD===<br />
<br />
There are many agile practices related to what drives the development of a project, what they have in common is that they could generate testing code. As a security practicioner, there are two aspects of interest. One, sometimes useful is called "test coverage" and it is automatically calculated. 100% code coverage does not mean a good coverage neither a 60% a bad one. It is very difficult to measure the second aspect, the quality<br />
of the test. There is possitive testing, that aims at the added value of every piece of software and negative testing, related to bugs and security.<br />
<br />
<br />
====Test Driven Development====<br />
<br />
<br />
TDD is the practice of making the test before coding, it is the extremme application of the "test early" principle. The idea is that the code always will be tested as the test predates the code itself. A very important side effect is that it forces to simplify the code to make it testeable. It could be very low level with very isolated components, called "unit testing" and high level, when it tests clusters of interrelated components, called "functional testing".<br />
<br />
To read more about code coverage: [https://www.owasp.org/index.php/Code_Review_Coverage]<br />
<br />
====Behavior Driven Development====<br />
<br />
This practice builds upon TDD, providing an interface to non-specialists users, shaping the tests around full blown scenarios. As TDD, it generates testing code. <br />
<br />
====Domain Driven Design====<br />
<br />
This practice consist of... It does not generate code itself, but the architecture. It is very popular among the Agile people, so it is very important to be familiar with. Perhaps the most useful concept is "ubiquitous language".</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_ClientSideCodeJScript&diff=175397CRV2 ClientSideCodeJScript2014-05-21T14:28:23Z<p>Carlos Pantelides: </p>
<hr />
<div><br />
JavaScript has several known security vulnerabilities, with HTML5 and JavaScript becoming more prevalent in web sites today and with more web sites moving to responsive web design with its dependence on JavaScript the code reviewer needs to understand what vulnerabilities to look for.<br />
<br />
The most significant vulnerabilities in JavaScript is cross-site scripting (XSS) and Document Object Model, DOM-based XSS.<br />
<br />
Detection of DOM-based XSS can be challenging. This is cause by the following reasons.<br />
<br />
* JavaScript is often obfuscated to protect intellectual property. <br />
* JavaScript is often compressed out of concerned for bandwidth.<br />
<br />
In both of these cases it is strongly recommended the code review be able to review the JavaScript before it has been obfuscated and or compressed. <br />
<br />
Another aspect that makes code review of JavaScript challenging is its reliance of large frameworks such as Microsoft .Net and Java Server Faces and the use of JavaScript frameworks, such as JQuery, Knockout, Angular, Backbone. These frameworks aggravate the problem because the code can only be fully analyzed given the source code of the framework itself. These frameworks are usually several orders of magnitude larger then the code the code reviewer needs to review. Because of time and money most companies simple accept that these frameworks are secure or the risks are low and acceptable to the organization.<br />
<br />
Because of these challenges we recommend a hybrid analysis for JavaScript. Manual source to sink validation when necessary, static analysis with black-box testing and taint testing. <br />
<br />
First use a static analysis. Code Reviewer and the organization needs to understand that because of event-driven behaviors, complex dependencies between HTML DOM and JavaScript code, and asynchronous communication with the server side static analysis will always fall short and may show both positive, false, false –positive, and positive-false findings.<br />
<br />
Black-box traditional methods detection of reflected or stored XSS needs to be preformed. However this approach will not work for DOM-based XSS vulnerabilities. <br />
<br />
Taint analysis needs to be incorporated into static analysis engine. Taint Analysis attempts to identify variables that have been 'tainted' with user controllable input and traces them to possible vulnerable functions also known as a 'sink'. If the tainted variable gets passed to a sink without first being sanitized it is flagged as vulnerability.<br />
<br />
Second the code reviewer needs to be certain the code was tested with JavaScript was turned off to make sure all client sided data validation was also validated on the server side.<br />
<br />
Code examples of JavaScript vulnerabilities.<br />
<br />
<html><br />
<script type=”text/javascript”><br />
var pos=document.URL.indexOf(“name=”)+5;<br />
document.write( document.URL.substring(pos,document.URL.length));<br />
</script><br />
<html><br />
<br />
Explanation: An attacker can send a link such as “http://hostname/welcome.html#name=<script>alert(1)</script> to the victim resulting in the victim’s browser executing the injected client-side code.<br />
<br />
# var url = document.location.url;<br />
# var loginIdx = url.indexOf(‘login’);<br />
# var loginSuffix = url.substring(loginIdx);<br />
# url = ‘http://mySite/html/sso/’ + loginSuffix;<br />
# document.location.url = url;<br />
<br />
Line 5 may be a false-positive and prove to be safe code or it may be open to “Open redirect attack” with taint analysis the static analysis should be able to correctly identified if this vulnerability exists. If static analysis relies only on black-box component this code will have flagged as vulnerable requiring the code reviewer to do a complete source to sink review. <br />
<br />
<br />
For Javascript and Session handling see [https://www.owasp.org/index.php/CRV2_SessionHandling Session Handling]<br />
<br />
<br />
References:<br />
# http://docstore.mik.ua/orelly/web/jscript/ch20_04.html<br />
# https://www.owasp.org/index.php/Static_Code_Analysis<br />
# http://www.cs.tau.ac.il/~omertrip/fse11/paper.pdf<br />
# http://www.jshint.com/about/<br />
# https://github.com/mozilla/doctorjs<br />
<br />
<br />
<br />
Three points of validity are required for Javascript codes:<br />
# Have all the logic server-side, Javascript is only the butler<br />
# Check for all sorts of XSS DOM Attacks<br />
# Check for insecure Javascript libraries and update them frequently.<br />
<br />
<br />
Javascript uses strings to create DOM elements. This can lead to XSS attacks. All input should be sanitized before being converted to DOM objects.<br />
<br />
Javascript libraries are not? prone to attack. Most of them have flaws in them, recent jQuery flaw (evaluating the document.location.hash, allowing XSS to be embedded after # in location) caused Drupal (which is generally a safe system) to allow admin user creation for attackers!</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_SessionHandling&diff=175396CRV2 SessionHandling2014-05-21T14:27:46Z<p>Carlos Pantelides: </p>
<hr />
<div>==General Considerations==<br />
# If the system is critical, Session IDs should be cryptographically secure (i.e non determinable)<br />
# In big systems, sessions should not be stored in files (default PHP behavior). They should be stored in memory or in databases, to prevent DOS attacks on new sessions.<br />
# As soon as a confidential or higher session is formed for a user, they should have all their traffic transmitted through SSL. SessionID is almost as important as passwords.<br />
# A policy should be defined and forced on an application, to define the number of sessions a user can have. (One, Many, etc.) If this is left vague, it usually leads to security flaws.<br />
# Sessions '''require''' a general timeout, which happens at a certain time after creation (usually a week), and an idle timeout, which happens after a certain time of the session being idle (usually 30 minutes).<br />
# The idle timeout can be changed depending on the nature of the application (smaller for banking applications, larger for email composing clients)<br />
# The idle timeout doesn't have to be precise. The application can check for it every 2 minutes, and flush all timed-out idle sessions.<br />
# Sessions should be rolled when they are elevated. Rolling means that the session-id should be changed, and the session information should be transferred to the new id.<br />
# Sessions need to be cleared out on logout. It is a good idea to dispose of the session-id on logout as well.<br />
<br />
<br />
<br />
==Session Attacks==<br />
Generally three sorts of session attacks are possible:<br />
# Session Hijacking: stealing someone's session-id, and using it to impersonate that user.<br />
# Session Fixation: setting someone's session-id to a predefined value, and impersonating them using that known value<br />
# Session Elevation: when the importance of a session is changed, but its ID is not.<br />
<br />
===Session Hijacking===<br />
<br />
# Mostly done via XSS attacks, mostly can be prevented by HTTP-Only session cookies (unless Javascript code requires access to them).<br />
# (charly proposes to eliminate this...) It's generally a good idea for Javascript not to need access to session cookies, as preventing all flavors of XSS is usually the toughest part of hardening a system.<br />
# Session-ids should be placed inside cookies, and not in URLs. URL informations are stored in browser's history, and HTTP Referrers, and can be accessed by attackers.<br />
# (...and add this) As cookies can be accessed by default from javascript and preventing all flavors of XSS is usually the toughest part of hardening a system, there is an attribute called "HTTPOnly", that forbids this access. The session cookie should has this attribute set. Anyway, as there is no need to access a session cookie from the client, you should get suspicious about client side code that depends on this access.<br />
# Geographical location checking can help detect simple hijacking scenarios. Advanced hijackers use the same IP (or range) of the victim.<br />
# An active session should be warned when it is accessed from another location.<br />
# An active users should be warned when s/he has an active session somewhere else (if the policy allows multiple sessions for a single user).<br />
<br />
===Session Fixation===<br />
# If the application sees a new session-id that is not present in the pool, it should be rejected and a new session-id should be advertised. This is the sole method to prevent fixation.<br />
# All the session-ids should be generated by the application, and then stored in a pool to be checked later for. Application is the sole authority for session generation.<br />
<br />
===Session Elevation===<br />
# Whenever a session is elevated (login, logout, certain authorization), it should be rolled.<br />
# Many applications create sessions for visitors as well (and not just authenticated users). They should definitely roll the session on elevation, because the user expects the application to treat them securely after they login.<br />
# When a down-elevation occurs, the session information regarding the higher level should be flushed.</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_ClientSideCodeJScript&diff=175395CRV2 ClientSideCodeJScript2014-05-21T12:21:54Z<p>Carlos Pantelides: </p>
<hr />
<div><br />
JavaScript has several known security vulnerabilities, with HTML5 and JavaScript becoming more prevalent in web sites today and with more web sites moving to responsive web design with its dependence on JavaScript the code reviewer needs to understand what vulnerabilities to look for.<br />
<br />
The most significant vulnerabilities in JavaScript is cross-site scripting (XSS) and Document Object Model, DOM-based XSS.<br />
<br />
Detection of DOM-based XSS can be challenging. This is cause by the following reasons.<br />
<br />
* JavaScript is often obfuscated to protect intellectual property. <br />
* JavaScript is often compressed out of concerned for bandwidth.<br />
<br />
In both of these cases it is strongly recommended the code review be able to review the JavaScript before it has been obfuscated and or compressed. <br />
<br />
Another aspect that makes code review of JavaScript challenging is its reliance of large frameworks such as Microsoft .Net and Java Server Faces and the use of JavaScript frameworks, such as JQuery, Knockout, Angular, Backbone. These frameworks aggravate the problem because the code can only be fully analyzed given the source code of the framework itself. These frameworks are usually several orders of magnitude larger then the code the code reviewer needs to review. Because of time and money most companies simple accept that these frameworks are secure or the risks are low and acceptable to the organization.<br />
<br />
Because of these challenges we recommend a hybrid analysis for JavaScript. Manual source to sink validation when necessary, static analysis with black-box testing and taint testing. <br />
<br />
First use a static analysis. Code Reviewer and the organization needs to understand that because of event-driven behaviors, complex dependencies between HTML DOM and JavaScript code, and asynchronous communication with the server side static analysis will always fall short and may show both positive, false, false –positive, and positive-false findings.<br />
<br />
Black-box traditional methods detection of reflected or stored XSS needs to be preformed. However this approach will not work for DOM-based XSS vulnerabilities. <br />
<br />
Taint analysis needs to be incorporated into static analysis engine. Taint Analysis attempts to identify variables that have been 'tainted' with user controllable input and traces them to possible vulnerable functions also known as a 'sink'. If the tainted variable gets passed to a sink without first being sanitized it is flagged as vulnerability.<br />
<br />
Second the code reviewer needs to be certain the code was tested with JavaScript was turned off to make sure all client sided data validation was also validated on the server side.<br />
<br />
Code examples of JavaScript vulnerabilities.<br />
<br />
<html><br />
<script type=”text/javascript”><br />
var pos=document.URL.indexOf(“name=”)+5;<br />
document.write( document.URL.substring(pos,document.URL.length));<br />
</script><br />
<html><br />
<br />
Explanation: An attacker can send a link such as “http://hostname/welcome.html#name=<script>alert(1)</script> to the victim resulting in the victim’s browser executing the injected client-side code.<br />
<br />
# var url = document.location.url;<br />
# var loginIdx = url.indexOf(‘login’);<br />
# var loginSuffix = url.substring(loginIdx);<br />
# url = ‘http://mySite/html/sso/’ + loginSuffix;<br />
# document.location.url = url;<br />
<br />
Line 5 may be a false-positive and prove to be safe code or it may be open to “Open redirect attack” with taint analysis the static analysis should be able to correctly identified if this vulnerability exists. If static analysis relies only on black-box component this code will have flagged as vulnerable requiring the code reviewer to do a complete source to sink review. <br />
<br />
<br />
==Javascript and Session handling==<br />
<br />
Session handling is almost always done with cookies. In order to protect from Session Hijack (see link), there is an attribute called "secure" (see Session Handling), that forbids the access to the session cookie from javascript. Anyway, as there is no need to access a session cookie from the client, you should get suspicious about client side code that depends on this access.<br />
<br />
<br />
References:<br />
# http://docstore.mik.ua/orelly/web/jscript/ch20_04.html<br />
# https://www.owasp.org/index.php/Static_Code_Analysis<br />
# http://www.cs.tau.ac.il/~omertrip/fse11/paper.pdf<br />
# http://www.jshint.com/about/<br />
# https://github.com/mozilla/doctorjs<br />
<br />
<br />
<br />
Three points of validity are required for Javascript codes:<br />
# Have all the logic server-side, Javascript is only the butler<br />
# Check for all sorts of XSS DOM Attacks<br />
# Check for insecure Javascript libraries and update them frequently.<br />
<br />
<br />
Javascript uses strings to create DOM elements. This can lead to XSS attacks. All input should be sanitized before being converted to DOM objects.<br />
<br />
Javascript libraries are not? prone to attack. Most of them have flaws in them, recent jQuery flaw (evaluating the document.location.hash, allowing XSS to be embedded after # in location) caused Drupal (which is generally a safe system) to allow admin user creation for attackers!</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_ClientSideCodeJScript&diff=175390CRV2 ClientSideCodeJScript2014-05-21T12:03:48Z<p>Carlos Pantelides: </p>
<hr />
<div><br />
JavaScript has several known security vulnerabilities, with HTML5 and JavaScript becoming more prevalent in web sites today and with more web sites moving to responsive web design with its dependence on JavaScript the code reviewer needs to understand what vulnerabilities to look for.<br />
<br />
The most significant vulnerabilities in JavaScript is cross-site scripting (XSS) and Document Object Model, DOM-based XSS.<br />
<br />
Detection of DOM-based XSS can be challenging. This is cause by the following reasons.<br />
<br />
* JavaScript is often obfuscated to protect intellectual property. <br />
* JavaScript is often compressed out of concerned for bandwidth.<br />
<br />
In both of these cases it is strongly recommended the code review be able to review the JavaScript before it has been obfuscated and or compressed. <br />
<br />
Another aspect that makes code review of JavaScript challenging is its reliance of large frameworks such as Microsoft .Net and Java Server Faces and the use of JavaScript frameworks, such as JQuery, Knockout, Angular, Backbone. These frameworks aggravate the problem because the code can only be fully analyzed given the source code of the framework itself. These frameworks are usually several orders of magnitude larger then the code the code reviewer needs to review. Because of time and money most companies simple accept that these frameworks are secure or the risks are low and acceptable to the organization.<br />
<br />
Because of these challenges we recommend a hybrid analysis for JavaScript. Manual source to sink validation when necessary, static analysis with black-box testing and taint testing. <br />
<br />
First use a static analysis. Code Reviewer and the organization needs to understand that because of event-driven behaviors, complex dependencies between HTML DOM and JavaScript code, and asynchronous communication with the server side static analysis will always fall short and may show both positive, false, false –positive, and positive-false findings.<br />
<br />
Black-box traditional methods detection of reflected or stored XSS needs to be preformed. However this approach will not work for DOM-based XSS vulnerabilities. <br />
<br />
Taint analysis needs to be incorporated into static analysis engine. Taint Analysis attempts to identify variables that have been 'tainted' with user controllable input and traces them to possible vulnerable functions also known as a 'sink'. If the tainted variable gets passed to a sink without first being sanitized it is flagged as vulnerability.<br />
<br />
Second the code reviewer needs to be certain the code was tested with JavaScript was turned off to make sure all client sided data validation was also validated on the server side.<br />
<br />
Code examples of JavaScript vulnerabilities.<br />
<br />
<html><br />
<script type=”text/javascript”><br />
var pos=document.URL.indexOf(“name=”)+5;<br />
document.write( document.URL.substring(pos,document.URL.length));<br />
</script><br />
<html><br />
<br />
Explanation: An attacker can send a link such as “http://hostname/welcome.html#name=<script>alert(1)</script> to the victim resulting in the victim’s browser executing the injected client-side code.<br />
<br />
# var url = document.location.url;<br />
# var loginIdx = url.indexOf(‘login’);<br />
# var loginSuffix = url.substring(loginIdx);<br />
# url = ‘http://mySite/html/sso/’ + loginSuffix;<br />
# document.location.url = url;<br />
<br />
Line 5 may be a false-positive and prove to be safe code or it may be open to “Open redirect attack” with taint analysis the static analysis should be able to correctly identified if this vulnerability exists. If static analysis relies only on black-box component this code will have flagged as vulnerable requiring the code reviewer to do a complete source to sink review. <br />
<br />
<br />
==Javascript and Session handling==<br />
<br />
Session handling is almost always done with cookies. In order to protect from Session Hijack (see link), there is an attribute called "secure" (see Session Handling), that forbids the access to the session cookie from javascript. Anyway, as there is no need to access a session cookie from the client, you should not make client side code that depends on this access.<br />
<br />
<br />
References:<br />
# http://docstore.mik.ua/orelly/web/jscript/ch20_04.html<br />
# https://www.owasp.org/index.php/Static_Code_Analysis<br />
# http://www.cs.tau.ac.il/~omertrip/fse11/paper.pdf<br />
# http://www.jshint.com/about/<br />
# https://github.com/mozilla/doctorjs<br />
<br />
<br />
<br />
Three points of validity are required for Javascript codes:<br />
# Have all the logic server-side, Javascript is only the butler<br />
# Check for all sorts of XSS DOM Attacks<br />
# Check for insecure Javascript libraries and update them frequently.<br />
<br />
<br />
Javascript uses strings to create DOM elements. This can lead to XSS attacks. All input should be sanitized before being converted to DOM objects.<br />
<br />
Javascript libraries are not? prone to attack. Most of them have flaws in them, recent jQuery flaw (evaluating the document.location.hash, allowing XSS to be embedded after # in location) caused Drupal (which is generally a safe system) to allow admin user creation for attackers!</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=164341CRV2 CodeReviewAgile2013-12-05T23:03:41Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between. There are many flavours of agile, perhaps as many as practicioners. It is like an heretogeneous reference framework and you are free to use what you want.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed. First, when the review is done and then, the code itself.<br />
<br />
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
===Life Cycle===<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity. Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the develop, test, code review cycle.<br />
<br />
It is a common practice to define short development cycles. At the end of each one, all the code must be production quality code. Its funtionality may be incomplete, but it must add some value. That affects the review process as it must be continuous.<br />
<br />
===Pair Programming===<br />
<br />
This technique is quite controversial, but when it is adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
===Peer Review===<br />
<br />
This one is enforced by the usage of tools like Jenkins that ask another user for a code review before commiting to the versioning system.<br />
<br />
===???===<br />
<br />
The role of testers... some people thinks that there is almost no place for the traditional tester role. Automated testing requires a tester to define the tests but not to execute them. This tester must be almost a programmer or work with a programmer.<br />
<br />
<br />
===Clean Code and "Smells"===<br />
<br />
The agile community is very committed to code quality. A by product is the concept of Clean Code and its smells, its very handy to be familiar with this topic, ...<br />
<br />
==The Agile Ecosystem==<br />
<br />
bdd----------------------------------------------------------------------+<br />
| |<br />
tdd-------------------------------------------------------------+ |<br />
| | |<br />
+----+--> pair programming --------+--> peer review -----> continuous integration<br />
| ( collective ownership ) |<br />
+--> individual programming ---+<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you. Or better yet, integrate yourself to the Team.<br />
<br />
Code review must be done at least at the end of every user story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review. Nevertheless, as security review requieres some extra specialization, perhaps the best way is to integrate the security code reviewier with the Team.<br />
<br />
==Tests==<br />
Agile projects tend to use a lot of automated testing, in order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
Some guide taken from ...<br />
*Are there enough tests?<br />
*Is the code covered by the tests? Code coverage measure main value is to find unused code.<br />
*Are there the trivial tests? They are as needed as any other test.<br />
*Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.<br />
*Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.<br />
*Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?<br />
*Are the test automatic? If the tests requiere manual intervention, they will not be run.<br />
*Do the tests conform to F.I.R.S.T.?<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
<br />
<br />
<br />
==Other Agile Resources==<br />
<br />
===The role of testing===<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
<br />
<br />
===Continuous integration===<br />
<br />
CI triggers the tests and often static code analysis too. It makes integration problems arise very quickly.<br />
<br />
<br />
===The role of automatic static code analysis in the Agile Methodologies===<br />
<br />
TBD<br />
<br />
===The XDD===<br />
<br />
There are many agile practices related to what drives the development of a project, what they have in common is that they could generate testing code. As a security practicioner, there are two aspects of interest. One, sometimes useful is called "test coverage" and it is automatically calculated. 100% code coverage does not mean a good coverage neither a 60% a bad one. It is very difficult to measure the second aspect, the quality<br />
of the test. There is possitive testing, that aims at the added value of every piece of software and negative testing, related to bugs and security.<br />
<br />
<br />
====Test Driven Development====<br />
<br />
<br />
TDD is the practice of making the test before coding, it is the extremme application of the "test early" principle. The idea is that the code always will be tested as the test predates the code itself. A very important side effect is that it forces to simplify the code to make it testeable. It could be very low level with very isolated components, called "unit testing" and high level, when it tests clusters of interrelated components, called "functional testing".<br />
<br />
To read more about code coverage: [https://www.owasp.org/index.php/Code_Review_Coverage]<br />
<br />
====Behavior Driven Development====<br />
<br />
This practice builds upon TDD, providing an interface to non-specialists users, shaping the tests around full blown scenarios. As TDD, it generates testing code. <br />
<br />
====Domain Driven Design====<br />
<br />
This practice consist of... It does not generate code itself, but the architecture. It is very popular among the Agile people, so it is very important to be familiar with. Perhaps the most useful concept is "ubiquitous language".<br />
<br />
===Refactoring===<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.<br />
<br />
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
<br />
==References and sources==<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
http://groups.yahoo.com/group/foro-agiles/<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://martinfowler.com/bliki/TestCoverage.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=160946CRV2 CodeReviewAgile2013-10-17T01:33:44Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between. There are many flavours of agile, perhaps as many as practicioners. It is like an heretogeneous reference framework and you are free to use what you want.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed. First, when the review is done and then, the code itself.<br />
<br />
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in theirself, in what traditionally was seem as another phase.<br />
<br />
===Life Cycle===<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity. Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the develop, test, code review cycle.<br />
<br />
It is a common practice to define short development cycles. At the end of each one, all the code must be production quality code. It can be incomplete, but it must add some value. That affects the review process as it must be continuous.<br />
<br />
===Pair Programming===<br />
<br />
This technique is quite controversial, but when it is adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
===Peer Review===<br />
<br />
This one is enforced by the usage of tools like Jenkins that ask another user for a code review before commiting to the versioning system.<br />
<br />
The role of testers...<br />
<br />
<br />
===Clean Code and "Smells"===<br />
<br />
The agile community is very committed to code quality...<br />
<br />
==The Agile Ecosystem==<br />
<br />
bdd----------------------------------------------------------------------+<br />
| |<br />
tdd-------------------------------------------------------------+ |<br />
| | |<br />
+----+--> pair programming --------+--> peer review -----> continuous integration<br />
| ( collective ownership ) |<br />
+--> individual programming ---+<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you. Or better yet, integrate to the Team.<br />
<br />
Code review must be done at least at the end of every user story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review. Nevertheless, as security review requieres some extra specialization, perhaps the best way is to integrate the security code reviewier with the Team.<br />
<br />
==Tests==<br />
Agile projects tend to use a lot of automated testing, in order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
Some guide taken from ...<br />
*Are there enough tests?<br />
*Is the code covered by the tests? Code coverage measure main value is to find unused code.<br />
*Are there the trivial tests? They are as needed as any other test.<br />
*Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.<br />
*Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.<br />
*Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?<br />
*Are the test automatic? If the tests requiere manual intervention, they will not be run.<br />
*Do the tests conform to F.I.R.S.T.?<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
<br />
<br />
<br />
==Other Agile Resources==<br />
<br />
===The role of testing===<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
<br />
<br />
===Continuous integration===<br />
it triggers the tests and often static code analysis too. It makes integration problems arise very quickly.<br />
<br />
<br />
===The role of automatic static code analysis in the Agile Methodologies===<br />
<br />
TBD<br />
<br />
===The XDD===<br />
<br />
There are many agile practices related to what drives the development of a project, what they have in common is that they could generate testing code. As a security practicioner, there are two aspects of interest. One, sometimes useful is called "test coverage" and it is automatically calculated. 100% code coverage does not mean a good coverage neither a 60% a bad one. It is very difficult to measure the second aspect, the quality<br />
of the test. There is possitive testing, that aims at the added value of every piece of software and negative testing, related to bugs and security.<br />
<br />
<br />
====Test Driven Development====<br />
<br />
<br />
TDD is the practice of making the test before coding, it is the extremme application of the "test early" principle. The idea is that the code always will be tested as the test predates the code itself. A very important side effect is that it forces to simplify the code to make it testeable. It could be very low level with very isolated components, called "unit testing" and high level, when it tests clusters of interrelated components, called "functional testing".<br />
<br />
To read more about code coverage: [https://www.owasp.org/index.php/Code_Review_Coverage]<br />
<br />
====Behavior Driven Development====<br />
<br />
This practice builds upon TDD, providing an interface to non-specialists users, shaping the tests around full blown scenarios. As TDD, it generates testing code. <br />
<br />
====Domain Driven Design====<br />
<br />
This practice consist of... It does not generate code itself, but the architecture. It is very popular among the Agile people, so it is very important to be familiar with. Perhaps the most useful concept is "ubiquitous language".<br />
<br />
===Refactoring===<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.<br />
<br />
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
<br />
==References and sources==<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
http://groups.yahoo.com/group/foro-agiles/<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://martinfowler.com/bliki/TestCoverage.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=158419CRV2 CodeReviewAgile2013-09-14T00:26:51Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between. There are many flavours of agile, perhaps as many as practicioners. It is like an heretogeneous reference framework and you are free to use what you want.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed. First, when the review is done and then the code itself.<br />
<br />
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
===Life Cycle===<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity. Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle.<br />
<br />
Its a common practice to define short development cycles. At the end of each one, all the code must be production quality code. It can be incomplete, but it must add some value. That affects the review process, it must be continuous.<br />
<br />
===Pair Programming===<br />
<br />
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
===Peer Review===<br />
<br />
This one is enforced by the usage of tools like Jenkins that ask another user for a code review before commiting to the versioning system.<br />
<br />
The role of testers...<br />
<br />
<br />
===Clean Code and "Smells"===<br />
<br />
The agile community is very committed to code quality...<br />
<br />
==The Agile Ecosystem==<br />
<br />
bdd----------------------------------------------------------------------+<br />
| |<br />
tdd-------------------------------------------------------------+ |<br />
| | |<br />
+----+--> pair programming --------+--> peer review -----> continuous integration<br />
| ( collective ownership ) |<br />
+--> individual programming ---+<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
Code review must be done at least at the end of every story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review. Nevertheless, as security review requieres some extra specialization, perhaps the best way is to integrate the security code reviewier with the Team.<br />
<br />
==Tests==<br />
Agile projects tend to use a lot of automated testing, in order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
Some guide taken from ...<br />
*Are there enough tests?<br />
*Is the code covered by the tests? Code coverage measure main value is to find unused code.<br />
*Are there the trivial tests? They are as needed as any other test.<br />
*Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.<br />
*Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.<br />
*Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?<br />
*Are the test automatic? If the tests requiere manual intervention, they will not be run.<br />
*Do the tests conform to F.I.R.S.T.?<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
<br />
<br />
<br />
==Other Agile Resources==<br />
<br />
===The role of testing===<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
<br />
<br />
===Continuous integration===<br />
it triggers the tests and often static code analysis too. It makes integration problems arise very quickly.<br />
<br />
<br />
===The role of automatic static code analysis in the Agile Methodologies===<br />
<br />
TBD<br />
<br />
===The XDD===<br />
<br />
There are many agile practices related to what drivens the development of a project, what they have in common is that they could generate testing code. As a security practicioner, there are two aspects of interest. One, sometimes useful is called "test coverage" and it is automatically calculated. 100% code coverage does not mean a good coverage neither a 60% a bad one. It is very difficult to measure the second aspect, the quality<br />
of the test. There is possitive testing, that aims at the added value of every piece of software and negative testing, related to bugs and security.<br />
<br />
<br />
====Test Driven Development====<br />
<br />
<br />
TDD is the practice of making the test before coding, it is the extremme application of the "test early" principle. The idea is that the code always will be tested as the test predates the code itself. A very important side effect is that it forces to simplify the code to make it testeable. It could be very low level with very isolated components, called "unit testing" and high level, when it tests clusters of interrelated components, called "functional testing".<br />
<br />
To read more about code coverage: [https://www.owasp.org/index.php/Code_Review_Coverage]<br />
<br />
====Behavior Driven Development====<br />
<br />
This practice builds upon TDD, providing an interface to non specialists users, shaping the tests around full blown scenarios. As TDD, it generates testing code. <br />
<br />
====Domain Driven Design====<br />
<br />
This practice consist of... It does not generate code itself, but the architecture.<br />
<br />
===Refactoring===<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.<br />
<br />
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
<br />
==References and sources==<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
http://groups.yahoo.com/group/foro-agiles/<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://martinfowler.com/bliki/TestCoverage.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=155422CRV2 CodeReviewAgile2013-07-12T05:46:10Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between. There are many flavours of agile, perhaps as many as practicioners. It is like an heretogeneous reference framework and you are free to use what you want.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed. First, when the review is done and then the code itself.<br />
<br />
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
===Life Cycle===<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity. Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle.<br />
<br />
Its a common practice to define short production cycles. At the end of each one, all the code must be production quality code. It can be incomplete, but it must add some value. That affects the review process, it must be continuous.<br />
<br />
===Pair Programming===<br />
<br />
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
===Peer Review===<br />
<br />
This one is enforced by the usage of tools like ¿Jenkins? that ask another user for a code review before commiting to the versioning system.<br />
<br />
The role of testers...<br />
<br />
<br />
<br />
<br />
===Clean Code and "Smells"===<br />
<br />
TBD<br />
<br />
==The Agile Ecosystem==<br />
<br />
bdd----------------------------------------------------------------------+<br />
| |<br />
tdd-------------------------------------------------------------+ |<br />
| | |<br />
+----+--> pair programming --------+--> peer review -----> continuous integration<br />
| ( collective ownership ) |<br />
+--> individual programming ---+<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
Code review must be done at least at the end of every story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review.<br />
<br />
In order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
*Are there enough tests?<br />
*Is the code covered by the tests? Code coverage measure main value is to find unused code.<br />
*Are there the trivial tests? They are as needed as any other test.<br />
*Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.<br />
*Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.<br />
*Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?<br />
*Are the test automatic? If the tests requiere manual intervention, they will not be run.<br />
*Do the tests conform to F.I.R.S.T.?<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
<br />
<br />
<br />
==Other Agile Resources==<br />
<br />
===The role of testing===<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
<br />
<br />
===Continuous integration===<br />
it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly.<br />
<br />
<br />
===The role of automatic static code analysis in the Agile Methodologies===<br />
<br />
TBD<br />
<br />
===The XDD===<br />
<br />
There are many agile practices related to what drivens the development of a project, what they have in common is that they could generate testing code. As a security practicioner, there are two aspects of interest. One, sometimes useful is called "test coverage" and is automatically calculated. 100% code coverage does not mean a good coverage neither a 60% a bad one. It is very difficult to measure the second aspect, the quality<br />
of the test. There is possitive testing, that aims at the added value of every piece of software and negative testing, related to bugs and security.<br />
<br />
<br />
====Test Driven Development====<br />
<br />
<br />
TDD is the practice of making the test before coding, it is the extremme application of the "test early" principle. The idea is that the code always will be tested as the test predates the code itself. A very important side effect is that it forces to simplify the code to make it testeable. It could be very low level with very isolated components, called "unit testing" and high level, when it tests clusters of interrelated components, called "functional testing".<br />
<br />
To read more about code coverage: [https://www.owasp.org/index.php/Code_Review_Coverage]<br />
<br />
====Behavior Driven Development====<br />
<br />
This practice builds upon TDD, providing an interface to non specialists users, shaping the tests around full blown scenarios. As TDD, it generates testing code. <br />
<br />
====Domain Driven Design====<br />
<br />
This practice consist of... It does not generate code itself, but patterns.<br />
<br />
===Refactoring===<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.<br />
<br />
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
<br />
<br />
<br />
<br />
==References and sources==<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
http://groups.yahoo.com/group/foro-agiles/<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://martinfowler.com/bliki/TestCoverage.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=155421CRV2 CodeReviewAgile2013-07-12T05:41:56Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between. There are many flavours of agile, perhaps as many as practicioners. It is like an heretogeneous reference framework and you are free to use what you want.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed. First, when the review is done and then the code itself.<br />
<br />
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
===Life Cycle===<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity. Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle.<br />
<br />
Its a common practice to define short production cycles. At the end of each one, all the code must be production quality code. It can be incomplete, but it must add some value. That affects the review process, it must be continuous.<br />
<br />
===Pair Programming===<br />
<br />
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
===Peer Review===<br />
<br />
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
The role of testers<br />
<br />
<br />
<br />
<br />
===Clean Code and "Smells"===<br />
<br />
==The Agile Ecosystem==<br />
<br />
bdd----------------------------------------------------------------------+<br />
| |<br />
tdd-------------------------------------------------------------+ |<br />
| | |<br />
+----+--> pair programming --------+--> peer review -----> continuous integration<br />
| ( collective ownership ) |<br />
+--> individual programming ---+<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
Code review must be done at least at the end of every story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review.<br />
<br />
In order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
Are there enough tests?<br />
<br />
Is the code covered by the tests? Code coverage measure main value is to find unused code.<br />
<br />
Are there the trivial tests? They are as needed as any other test.<br />
<br />
Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.<br />
<br />
Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.<br />
<br />
Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?<br />
<br />
Are the test automatic? If the tests requiere manual intervention, they will not be run.<br />
<br />
Do the tests conform to F.I.R.S.T.?<br />
<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
<br />
<br />
<br />
==Other Agile Resources==<br />
<br />
===The role of testing===<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
<br />
<br />
===Continuous integration===<br />
it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly.<br />
<br />
<br />
===The role of automatic static code analysis in the Agile Methodologies===<br />
<br />
===The XDD===<br />
<br />
There are many agile practices related to what drivens the development of a project, what they have in common is that they could generate testing code. As a security practicioner, there are two aspects of interest. One, sometimes useful is called "test coverage" and is automatically calculated. 100% code coverage does not mean a good coverage neither a 60% a bad one. It is very difficult to measure the second aspect, the quality<br />
of the test. There is possitive testing, that aims at the added value of every piece of software and negative testing, related to bugs and security.<br />
<br />
<br />
====Test Driven Development====<br />
<br />
<br />
TDD is the practice of making the test before coding, it is the extremme application of the "test early" principle. The idea is that the code always will be tested as the test predates the code itself. A very important side effect is that it forces to simplify the code to make it testeable. It could be very low level with very isolated components, called "unit testing" and high level, when it tests clusters of interrelated components, called "functional testing".<br />
<br />
To read more about code coverage: [https://www.owasp.org/index.php/Code_Review_Coverage]<br />
<br />
====Behavior Driven Development====<br />
<br />
This practice builds upon TDD, providing an interface to non specialists users, shaping the tests around full blown scenarios. As TDD, it generates testing code. <br />
<br />
====Domain Driven Design====<br />
<br />
This practice consist of... It does not generate code itself, but patterns.<br />
<br />
===Refactoring===<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.<br />
<br />
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
<br />
<br />
<br />
<br />
==References and sources==<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
http://groups.yahoo.com/group/foro-agiles/<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://martinfowler.com/bliki/TestCoverage.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=155419CRV2 CodeReviewAgile2013-07-12T05:39:19Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between. There are many flavours of agile, perhaps as many as practicioners. It is like an heretogeneous reference framework and you are free to use what you want.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed. First, when the review is done and then the code itself.<br />
<br />
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
===Life Cycle===<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity. Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle.<br />
<br />
Its a common practice to define short production cycles. At the end of each one, all the code must be production quality code. It can be incomplete, but it must add some value. That affects the review process, it must be continuous.<br />
<br />
===Pair Programming===<br />
<br />
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
===Peer Review===<br />
<br />
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
The role of testers<br />
<br />
<br />
<br />
<br />
===Clean Code and "Smells"===<br />
<br />
==The Agile Ecosystem==<br />
<br />
bdd----------------------------------------------------------------------+<br />
| |<br />
tdd-------------------------------------------------------------+ |<br />
| | |<br />
+----+--> pair programming --------+--> peer review -----> continuous integration<br />
| ( collective ownership ) |<br />
+--> individual programming ---+<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
Code review must be done at least at the end of every story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review.<br />
<br />
In order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
Are there enough tests?<br />
<br />
Is the code covered by the tests? Code coverage measure main value is to find unused code.<br />
<br />
Are there the trivial tests? They are as needed as any other test.<br />
<br />
Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.<br />
<br />
Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.<br />
<br />
Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?<br />
<br />
Are the test automatic? If the tests requiere manual intervention, they will not be run.<br />
<br />
Do the tests conform to F.I.R.S.T.?<br />
<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
<br />
<br />
<br />
==Other Agile Resources==<br />
<br />
===The role of testing===<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
<br />
<br />
===Continuous integration===<br />
it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly.<br />
<br />
<br />
===The role of automatic static code analysis in the Agile Methodologies===<br />
<br />
===The XDD===<br />
<br />
There are many agile practices related to what drivens the development of a project, what they have in common is that they could generate testing code. As a security practicioner, there are two aspects of interest. One, sometimes useful is called "test coverage" and is automatically calculated. 100% code coverage does not mean a good coverage neither a 60% a bad one. It is very difficult to measure the second aspect, the quality<br />
of the test. There is possitive testing, that aims at the added value of every piece of software and negative testing, related to bugs and security.<br />
<br />
<br />
====Test Driven Development====<br />
<br />
<br />
TDD is the practice of making the test before coding, it is the extremme application of the "test early" principle. The idea is that the code always will be tested as the test predates the code itself. A very important side effect is that it forces to simplify the code to make it testeable. It could be very low level with very isolated components, called "unit testing" and high level, when it tests clusters of interrelated components, called "functional testing".<br />
<br />
To read more about code coverage: https://www.owasp.org/index.php/Code_Review_Coverage<br />
<br />
====Behavior Driven Development====<br />
<br />
This practice builds upon TDD, providing an interface to non specialists users, shaping the tests around full blown scenarios. As TDD, it generates testing code. <br />
<br />
====Domain Driven Design====<br />
<br />
This practice consist of... It does not generate code, but high level patterns.<br />
<br />
===Refactoring===<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.<br />
<br />
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
<br />
<br />
<br />
<br />
==References and sources==<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
http://groups.yahoo.com/group/foro-agiles/<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://martinfowler.com/bliki/TestCoverage.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=154346CRV2 CodeReviewAgile2013-06-23T23:35:42Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between. There are many flavours of agile, perhaps as many as practicioners. It is like an heretogeneous reference framework and you are free to use what you want.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed. First, when the review is done and then the code itself.<br />
<br />
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
===Life Cycle===<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity. Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle.<br />
<br />
Its a common practice to define short production cycles. At the end of each one, all the code must be production quality code. It can be incomplete, but it must add some value. That affects the review process, it must be continuous.<br />
<br />
===Pair Programming===<br />
<br />
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
===Peer Review===<br />
<br />
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
The role of testers<br />
<br />
<br />
<br />
<br />
===Clean Code and "Smells"===<br />
<br />
==The Agile Ecosystem==<br />
<br />
bdd----------------------------------------------------------------------+<br />
| |<br />
tdd-------------------------------------------------------------+ |<br />
| | |<br />
+----+--> pair programming --------+--> peer review -----> continuous integration<br />
| ( collective ownership ) |<br />
+--> individual programming ---+<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
Code review must be done at least at the end of every story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review.<br />
<br />
In order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
Are there enough tests?<br />
<br />
Is the code covered by the tests? Code coverage measure main value is to find unused code.<br />
<br />
Are there the trivial tests? They are as needed as any other test.<br />
<br />
Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.<br />
<br />
Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.<br />
<br />
Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?<br />
<br />
Are the test automatic? If the tests requiere manual intervention, they will not be run.<br />
<br />
Do the tests conform to F.I.R.S.T.?<br />
<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
<br />
<br />
<br />
==Other Agile Resources==<br />
<br />
===The role of testing===<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
<br />
<br />
===Continuous integration===<br />
it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly.<br />
<br />
<br />
===The role of automatic static code analysis in the Agile Methodologies===<br />
<br />
<br />
<br />
===Test Driven Development===<br />
<br />
It aims at code simplicity due to the need of making it testeable<br />
<br />
===Behavior Driven Development===<br />
<br />
....<br />
<br />
===Domain Driven Design===<br />
<br />
....<br />
<br />
===Refactoring===<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.<br />
<br />
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
<br />
<br />
<br />
<br />
==References and sources==<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
http://groups.yahoo.com/group/foro-agiles/<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://martinfowler.com/bliki/TestCoverage.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=154345CRV2 CodeReviewAgile2013-06-23T23:25:58Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed.<br />
<br />
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity.<br />
<br />
<br />
<br />
===Pair Programming===<br />
<br />
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
==Peer Review==<br />
<br />
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
===Life Cycle===<br />
<br />
Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle.<br />
<br />
<br />
<br />
<br />
===Clean Code and "Smells"===<br />
<br />
==The Agile Ecosystem==<br />
<br />
bdd----------------------------------------------------------------------+<br />
| |<br />
tdd-------------------------------------------------------------+ |<br />
| | |<br />
+----+--> pair programming --------+--> peer review -----> continuous integration<br />
| ( collective ownership ) |<br />
+--> individual programming ---+<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
Code review must be done at least at the end of every story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review.<br />
<br />
In order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
Are there enough tests?<br />
<br />
Is the code covered by the tests? Code coverage measure main value is to find unused code.<br />
<br />
Are there the trivial tests? They are as needed as any other test.<br />
<br />
Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.<br />
<br />
Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.<br />
<br />
Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?<br />
<br />
Are the test automatic? If the tests requiere manual intervention, they will not be run.<br />
<br />
Do the tests conform to F.I.R.S.T.?<br />
<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
<br />
<br />
<br />
==Other Agile Resources==<br />
<br />
===The role of testing===<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
<br />
<br />
===Continuous integration===<br />
it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly.<br />
<br />
<br />
===The role of automatic static code analysis in the Agile Methodologies===<br />
<br />
<br />
<br />
===Test Driven Development===<br />
<br />
It aims at code simplicity due to the need of making it testeable<br />
<br />
===Behavior Driven Development===<br />
<br />
....<br />
<br />
===Domain Driven Design===<br />
<br />
....<br />
<br />
===Refactoring===<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.<br />
<br />
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
<br />
<br />
<br />
<br />
==References and sources==<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
http://groups.yahoo.com/group/foro-agiles/<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://martinfowler.com/bliki/TestCoverage.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=154344CRV2 CodeReviewAgile2013-06-23T23:24:04Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed.<br />
<br />
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity.<br />
<br />
<br />
<br />
==Pair Programming==<br />
<br />
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
==Peer Review==<br />
<br />
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
==Life Cycle==<br />
<br />
Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle.<br />
<br />
<br />
<br />
<br />
<br />
==Clean Code and "Smells"==<br />
<br />
==The Agile Ecosystem==<br />
<br />
bdd----------------------------------------------------------------------+<br />
| |<br />
tdd-------------------------------------------------------------+ |<br />
| | |<br />
+----+--> pair programming --------+--> peer review -----> continuous integration<br />
| ( collective ownership ) |<br />
+--> individual programming ---+<br />
<br />
<br />
==Agile Resources==<br />
<br />
===The role of testing===<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
<br />
<br />
===Continuous integration===<br />
it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly.<br />
<br />
<br />
===The role of automatic static code analysis in the Agile Methodologies===<br />
<br />
<br />
<br />
===Test Driven Development===<br />
<br />
It aims at code simplicity due to the need of making it testeable<br />
<br />
===Behavior Driven Development===<br />
<br />
....<br />
<br />
===Domain Driven Design===<br />
<br />
....<br />
<br />
===Refactoring===<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.<br />
<br />
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
Code review must be done at least at the end of every story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review.<br />
<br />
In order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
Are there enough tests?<br />
<br />
Is the code covered by the tests? Code coverage measure main value is to find unused code.<br />
<br />
Are there the trivial tests? They are as needed as any other test.<br />
<br />
Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.<br />
<br />
Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.<br />
<br />
Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?<br />
<br />
Are the test automatic? If the tests requiere manual intervention, they will not be run.<br />
<br />
Do the tests conform to F.I.R.S.T.?<br />
<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
<br />
<br />
<br />
<br />
<br />
References and sources:<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
http://groups.yahoo.com/group/foro-agiles/<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://martinfowler.com/bliki/TestCoverage.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=154343CRV2 CodeReviewAgile2013-06-23T23:15:40Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed.<br />
<br />
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity.<br />
<br />
<br />
<br />
==Pair Programming==<br />
<br />
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
==Peer Review==<br />
<br />
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
==Life Cycle==<br />
<br />
Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle.<br />
<br />
<br />
<br />
<br />
<br />
==Clean Code and "Smells"==<br />
<br />
==Agile resources==<br />
<br />
===The role of testing===<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
<br />
<br />
===Continuous integration===<br />
it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly.<br />
<br />
<br />
===The role of automatic static code analysis in the Agile Methodologies===<br />
<br />
<br />
<br />
===Test Driven Development===<br />
<br />
It aims at code simplicity due to the need of making it testeable<br />
<br />
===Behavior Driven Development===<br />
<br />
....<br />
<br />
===Domain Driven Design===<br />
<br />
....<br />
<br />
===Refactoring===<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.<br />
<br />
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
Code review must be done at least at the end of every story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review.<br />
<br />
In order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
Are there enough tests?<br />
<br />
Is the code covered by the tests? Code coverage measure main value is to find unused code.<br />
<br />
Are there the trivial tests? They are as needed as any other test.<br />
<br />
Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.<br />
<br />
Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.<br />
<br />
Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?<br />
<br />
Are the test automatic? If the tests requiere manual intervention, they will not be run.<br />
<br />
Do the tests conform to F.I.R.S.T.?<br />
<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
<br />
<br />
<br />
<br />
<br />
References and sources:<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
http://groups.yahoo.com/group/foro-agiles/<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://martinfowler.com/bliki/TestCoverage.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=154200CRV2 CodeReviewAgile2013-06-20T21:30:12Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed.<br />
<br />
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity.<br />
<br />
<br />
<br />
==Pair Programming==<br />
<br />
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
==Peer Review==<br />
<br />
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
==Life Cycle==<br />
<br />
Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle.<br />
<br />
<br />
<br />
<br />
<br />
==Clean Code and "Smells"==<br />
<br />
==The role of testing==<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
==Continuous integration==<br />
it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly.<br />
<br />
<br />
==The role of automatic static code analysis in the Agile Methodologies==<br />
<br />
==Test Driven Development==<br />
<br />
It aims at code simplicity due to the need of making it testeable<br />
<br />
==Behavior Driven Development==<br />
<br />
....<br />
<br />
==Domain Driven Design==<br />
<br />
....<br />
<br />
==Refactoring==<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. The problem is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
In order to review an Agile Project, you will have to extend the review to the tests.<br />
<br />
Are there enough tests?<br />
<br />
Is the code covered by the tests? <br />
<br />
Are there the trivial tests?<br />
<br />
Are there commented out tests?<br />
<br />
Are boundary conditions tested? <br />
<br />
Are bugs exhaustively tested?<br />
<br />
Are the test automatic?<br />
<br />
Do the tests conform to F.I.R.S.T.?<br />
<br />
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.<br />
<br />
Independent: one test can not depend on the execution of another one, the order should not matter.<br />
<br />
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.<br />
<br />
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.<br />
<br />
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.<br />
<br />
<br />
<br />
<br />
<br />
<br />
References and sources:<br />
<br />
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
not used http://refactoring.com/<br />
<br />
not used http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=154143CRV2 CodeReviewAgile2013-06-20T02:08:32Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed.<br />
<br />
Agile Development is well suited for code review, as two of its key practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.<br />
<br />
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity.<br />
<br />
<br />
<br />
==Pair Programming==<br />
<br />
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
==Peer Review==<br />
<br />
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
==Life Cycle==<br />
AD tries to keep the code review as near as possible to the development phase, there is no such thing as develop, test, code review cycle.<br />
<br />
<br />
<br />
<br />
<br />
==Clean Code and "Smells"==<br />
<br />
==The role of testing==<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
==Continuous integration==<br />
it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly.<br />
<br />
<br />
==The role of automatic static code analysis in the Agile Methodologies==<br />
<br />
==Test Driven Development==<br />
<br />
It aims at code simplicity due to the need of making it testeable<br />
<br />
==Behavior Driven Development==<br />
<br />
....<br />
<br />
==Domain Driven Design==<br />
<br />
....<br />
<br />
==Refactoring==<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. The problem is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
<br />
==Code Reviewing an Agile Project==<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
In order to review an Agile Project, you will have to extend the review to the tests and ...<br />
<br />
<br />
<br />
References:<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html<br />
<br />
http://refactoring.com/<br />
<br />
http://dddcommunity.org/</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=154142CRV2 CodeReviewAgile2013-06-20T02:00:02Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management.<br />
<br />
Agile has some key practices that could affect the way the code is reviewed.<br />
<br />
Agile Development is well suited for code review, as two of its key practices are "pair programming" and "peer review". AD incorporates code review in itself<br />
<br />
Agile blurs the difference between developing and testing, and so it does with code review. It is not an external activity.<br />
<br />
<br />
<br />
==Pair Programming==<br />
<br />
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.<br />
<br />
==Peer Review==<br />
<br />
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
==Life Cycle==<br />
AD tries to keep the code review as near as possible to the development phase, there is no such thing as develop, test, code review cycle.<br />
<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
<br />
==Clean Code and "Smells"==<br />
<br />
==The role of testing==<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
==continuous integration==<br />
it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly.<br />
<br />
<br />
==The role of automatic static code analysis in the Agile Methodologies==<br />
<br />
==Test Driven Development==<br />
<br />
It aims at code simplicity due to the need of making it testeable<br />
<br />
==Behavior Driven Development==<br />
<br />
....<br />
<br />
==Domain Driven Design==<br />
<br />
....<br />
<br />
==Refactoring==<br />
<br />
Refactoring is the art of changing the code with out changing its behavior. The problem is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.<br />
<br />
References:<br />
<br />
http://martinfowler.com/articles/continuousIntegration.html</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=154138CRV2 CodeReviewAgile2013-06-20T00:58:22Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
==Some definitions about Agile==<br />
<br />
...<br />
<br />
Agile Development is well suited for code review, as two of its best practices are "peer programming" and "peer review". AD incorporates code review in itself <br />
<br />
==Peer Programming==<br />
<br />
This technique consists of ...<br />
<br />
==Peer Review==<br />
<br />
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
==LifeCycle==<br />
AD tries to keep the code review as near as possible to the development phase, there is no such thing as develop, test, code review cycle.<br />
<br />
<br />
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.<br />
<br />
<br />
==Clean Code and "Smells"==<br />
<br />
==The role of testing==<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
==continuous integration==<br />
it can trigger static code analysis<br />
<br />
<br />
==The role of automatic static code analysis in the Agile Methodologies==<br />
<br />
==Test Driven Development==<br />
<br />
It aims at code simplicity due to the need of making it testeable<br />
<br />
==Behavior Driven Development==<br />
<br />
....<br />
<br />
==Domaing Driven Design==<br />
<br />
....</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=154137CRV2 CodeReviewAgile2013-06-20T00:50:23Z<p>Carlos Pantelides: </p>
<hr />
<div>Charly, remember that it's "code review guide", not "testing guide"<br />
<br />
<br />
=Some definitions about Agile=<br />
<br />
...<br />
<br />
Agile Development is well suited for code review, as two of its best practices are "peer programming" and "peer review". AD incorporates code review in itself <br />
<br />
==Peer Programming==<br />
<br />
This technique consists of ...<br />
<br />
==Peer Review==<br />
<br />
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
==LifeCycle==<br />
AD tries to keep the code review as near as possible to the development phase, there is no such thing as develop, test, code review cycle.<br />
<br />
<br />
==Clean Code and "Smells"==<br />
<br />
==The role of testing==<br />
<br />
It is so fundamental, that the xDD pervades Agile, test first, test earlier <br />
<br />
==continuous integration==<br />
it can trigger static code analysis<br />
<br />
<br />
==The role of automatic static code analysis in the Agile Methodologies==<br />
<br />
==Test Driven Development==<br />
<br />
It aims at code simplicity due to the need of making it testeable<br />
<br />
==Behavior Driven Development==<br />
<br />
....<br />
<br />
==Domaing Driven Design==<br />
<br />
....</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=CRV2_CodeReviewAgile&diff=154135CRV2 CodeReviewAgile2013-06-20T00:26:23Z<p>Carlos Pantelides: Created page with "Agile Development is well suited for code review, as two of its best practices are "peer programming" and "peer review". The fis The last one is enforced by the usage of too..."</p>
<hr />
<div>Agile Development is well suited for code review, as two of its best practices are "peer programming" and "peer review". <br />
<br />
The fis<br />
The last one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.<br />
<br />
AD tries to keep the code review as near as possible to the development phase, there is no such thing as develop, test, code review cycle.</div>Carlos Pantelideshttps://wiki.owasp.org/index.php?title=OWASP_Code_Review_V2_Table_of_Contents&diff=154134OWASP Code Review V2 Table of Contents2013-06-20T00:22:47Z<p>Carlos Pantelides: </p>
<hr />
<div><br />
= '''OWASP Code Review Guide v2.0:''' =<br />
<br />
==Forward==<br />
# Author - Eoin Keary<br />
# Previous version to be updated:[[https://www.owasp.org/index.php/Code_Review_Guide_History]]<br />
'''[[CRV2_Forward|Content here]]'''<br />
<br />
== Code Review Guide Introduction==<br />
# Author - Eoin Keary<br />
# Previous version to be updated:[[https://www.owasp.org/index.php/Code_Review_Introduction]]<br />
'''[[CRV2_Introduction|Content here]]'''<br />
<br />
=== What is source code review and Static Analysis ===<br />
# Author - Zyad Mghazli<br />
# New Section<br />
''' [[CRV2_WhatIsCodeReview|Content here]]'''<br />
<br />
=== Manual Review - Pros and Cons ===<br />
# Author - Ashish Rao<br />
# New Section<br />
# Suggestion: Benchmark of different Stataic Analysis Tools Zyad Mghazli<br />
# Suggestion: Highlight the advantages of code review to the department/team - Gary David Robinson<br />
# [[CRV2_ManualReviewProsCons|Put content here]]<br />
<br />
=== Why code review ===<br />
==== Scope and Objective of secure code review ====<br />
# Author - Ashish Rao<br />
# [[CRV2_WhyCodeReview|Put content here]]<br />
<br />
=== We can't hack ourselves secure ===<br />
# Author - Prathamesh Mhatre<br />
# New Section<br />
# [[CRV2_CantHackSecure|Put content here]]<br />
<br />
=== 360 Review: Coupling source code review and Testing / Hybrid Reviews===<br />
# Author - Ashish Rao<br />
# New Section<br />
# [[CRV2_360Review|Put content here]]<br />
<br />
=== Can static code analyzers do it all? ===<br />
# Author - Ashish Rao<br />
# New Section<br />
# [[CRV2_CanStaticAnalyzersDoAll|Put content here]]<br />
<br />
=Methodology=<br />
===The code review approach===<br />
#Author - Prathamesh Mhatre<br />
# [[CRV2_CodeReviewApproach|Put content here]]<br />
<br />
==== Preparation and context ====<br />
# Author - Open<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Code_Review_Preparation]]<br />
# [[CRV2_PrepContext|Put content here]]<br />
<br />
====Application Threat Modeling====<br />
#Author - Andy, Renchie Joan<br />
# Previous version to be updated: [[https://www.owasp.org/OCRG1.1:Application_Threat_Modeling]]<br />
# [[CRV2_AppThreatModeling|Put content here]]<br />
<br />
====Understanding Code layout/Design/Architecture====<br />
#Author - Ashish Rao<br />
# [[CRV2_CodeLayoutDesignArch|Put content here]]<br />
<br />
===SDLC Integration===<br />
#Author - Andy, Ashish Rao<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Security_Code_Review_in_the_SDLC]]<br />
# [[CRV2_SDLCInt|Put content here]]<br />
<br />
====Deployment Models====<br />
=====Secure deployment configurations=====<br />
#Author - Ashish Rao<br />
# [[CRV2_SecDepConfig|Put content here]]<br />
<br />
# New Section<br />
=====Metrics and code review=====<br />
#Author - Andy<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Code_Review_Metrics]]<br />
# [[CRV2_MetricsCodeRev|Put content here]]<br />
<br />
=====Source and sink reviews=====<br />
#Author - Ashish Rao<br />
# New Section<br />
# [[CRV2_SourceSinkRev|Put content here]]<br />
<br />
=====Code review Coverage=====<br />
#Author - Open<br />
#Previous version to be updated: [[https://www.owasp.org/index.php/Code_Review_Coverage]]<br />
# [[CRV2_CodeRevCoverage|Put content here]]<br />
<br />
=====Design Reviews=====<br />
#Author - Ashish Rao<br />
*Why to review design?<br />
**Building security in design - secure by design principle<br />
**Design Areas to be reviewed<br />
**Common Design Flaws<br />
# [[CRV2_DesignRev|Put content here]]<br />
<br />
=====A Risk based approach to code review=====<br />
#Author - Renchie Joan<br />
#New Section<br />
*"Doing things right or doing the right things..."<br />
**"Not all bugs are equal<br />
# [[CRV2_RiskBasedApproach|Put content here]]<br />
<br />
====Crawling code====<br />
#Author - Abbas Naderi<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Crawling_Code]]<br />
*API of Interest:<br />
**Java<br />
**.NET<br />
**PHP<br />
**RUBY<br />
*Frameworks:<br />
**Spring<br />
**.NET MVC<br />
**Structs<br />
**Zend<br />
#New Section<br />
*Searching for code in C/C++<br />
#Author - Gary Robinson<br />
<br />
# [[CRV2_CrawlingCode|Put content here]]<br />
<br />
====Code reviews and Compliance====<br />
#Author -Manual Harti<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Code_Reviews_and_Compliance]]<br />
# [[CRV2_CodeRevCompliance|Put content here]]<br />
<br />
=Reviewing by Techincal Control=<br />
===Reviewing code for Authentication controls===<br />
#Author - Anand Prakash, Joan Renchie<br />
# [[CRV2_AuthControls|Put content here]]<br />
<br />
====Forgot password====<br />
#Author Abbas Naderi<br />
# [[CRV2_ForgotPassword|Put content here]]<br />
<br />
====Authentication====<br />
#Author - Anand Prakash, Joan Renchie<br />
# [[CRV2_Authentication|Put content here]]<br />
<br />
====CAPTCHA====<br />
#Author Larry Conklin, Joan Renchie<br />
'''[[CRV2_CAPTCHA|Content here]]'''<br />
<br />
====Out of Band considerations====<br />
#Author - Open<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Codereview-Authentication]]<br />
# [[CRV2_OutofBand|Put content here]]<br />
<br />
===Reviewing code Authorization weakness===<br />
#Author Ashish Rao<br />
# [[CRV2_AuthorizationWeaknesses|Put content here]]<br />
<br />
====Checking authz upon every request====<br />
#Author - Abbas Naderi, Joan Renchie<br />
# [[CRV2_CheckAuthzEachRequest|Put content here]]<br />
<br />
====Reducing the attack surface====<br />
#Author Chris Berberich<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Codereview-Authorization]]<br />
# [[CRV2_ReducingAttSurf|Put content here]]<br />
<br />
====Reviewing code for Session handling====<br />
#Author - Palak Gohil, Abbas Naderi<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Codereview-Session-Management]]<br />
# [[CRV2_SessionHandling|Put content here]]<br />
<br />
====Reviewing client side code====<br />
#New Section<br />
# [[CRV2_ClientSideCodeIntro|Put content here]]<br />
<br />
=====Javascript=====<br />
#Author - Abbas Naderi<br />
# [[CRV2_ClientSideCodeJScript|Put content here]]<br />
<br />
=====JSON=====<br />
#Author - Open<br />
# [[CRV2_ClientSideCodeJSon|Put content here]]<br />
<br />
=====Content Security Policy=====<br />
#Author - Open<br />
# [[CRV2_ClientSideCodeContSecPolicy|Put content here]]<br />
<br />
====="Jacking"/Framing=====<br />
#Author - Abbas Naderi<br />
# [[CRV2_ClientSideCodeJackingFraming|Put content here]]<br />
<br />
=====HTML 5?=====<br />
#Author - Sebastien Gioria<br />
# [[CRV2_ClientSideCodeHTML5|Put content here]]<br />
<br />
=====Browser Defenses policy=====<br />
#Author - Open<br />
# [[CRV2_ClientSideCodeBrowserDefPol|Put content here]]<br />
<br />
=====etc...=====<br />
<br />
====Review code for input validation====<br />
#Author - Open<br />
# [[CRV2_InputValIntro|Put content here]]<br />
<br />
=====Regex Gotchas=====<br />
#Author - Abbas Naderi<br />
#New Section<br />
# [[CRV2_InputValRegexGotchas|Put content here]]<br />
<br />
=====ESAPI=====<br />
#Author - Abbas Naderi<br />
#New Section<br />
# Internal Link: [[https://www.owasp.org/index.php/Codereview-Input_Validation]]<br />
# [[CRV2_InputValESAPI|Put content here]]<br />
<br />
====Reviewing code for contextual encoding====<br />
=====HTML Attribute=====<br />
#Author - Shenai Silva<br />
# [[CRV2_ContextEncHTMLAttribute|Put content here]]<br />
<br />
=====HTML Entity=====<br />
#Author - Shenai Silva<br />
# [[CRV2_ContextEncHTMLEntity|Put content here]]<br />
<br />
=====Javascript Parameters=====<br />
#Author - Open<br />
# [[CRV2_ContextEncJscriptParams|Put content here]]<br />
<br />
=====JQuery=====<br />
#Author - Abbas Naderi<br />
# [[CRV2_ContextEncJQuery|Put content here]]<br />
<br />
====Reviewing file and resource handling code====<br />
#Author - Open<br />
# [[CRV2_FileResourceHandling|Put content here]]<br />
<br />
====Resource Exhaustion - error handling====<br />
#Author - Abbas Naderi<br />
# [[CRV2_ResourceExhaustionErrHandling|Put content here]]<br />
<br />
=====native calls=====<br />
#Author Abbas Naderi<br />
# [[CRV2_ResourceExhaustionNativeCalls|Put content here]]<br />
<br />
====Reviewing Logging code - Detective Security====<br />
#Author - Palak Gohil<br />
* Where to Log<br />
* What to log<br />
* What not to log<br />
* How to log<br />
# Internal link: [[https://www.owasp.org/index.php/Logging_Cheat_Sheet]]<br />
# [[CRV2_LoggingCode|Put content here]]<br />
<br />
====Reviewing Error handling and Error messages====<br />
#Author - Gary Robinson<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Codereview-Error-Handling]]<br />
# [[CRV2_ErrorHandlingMessages|Put content here]]<br />
<br />
====Reviewing Security alerts====<br />
#Author - Open<br />
# [[CRV2_SecurityAlerts|Put content here]]<br />
<br />
====Review for active defense====<br />
#Author - Colin Watson<br />
# [[CRV2_ActiveDefense|Put content here]]<br />
<br />
====Reviewing Secure Storage====<br />
#Author - Azzeddine Ramrami<br />
# New Section<br />
# [[CRV2_SecureStorage|Put content here]]<br />
<br />
====Hashing & Salting - When, How and Where====<br />
=====Encrpyption=====<br />
======.NET======<br />
#Author Larry Conklin, Joan Renchie<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Codereview-Cryptographic_Controls]]<br />
*''Can we talk about key storage as well i.e. key management for encryption techniques used in the application? - Ashish Rao''<br />
'''[[CRV2_HashingandSaltingdotNet|Content here]]'''<br />
<br />
=Reviewing by Vulnerability=<br />
===Review Code for XSS===<br />
#Author Palak Gohil, Anand Prakash<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Reviewing_Code_for_Cross-Site_Scripting]]<br />
# In reviewing code for XSS - we can give more patterns on "source to sink" patterns for ASP.NET wrf to difference versions and mechanisms to display data in a page - Ashish Rao<br />
# [[CRV2_RevCodeXSS|Put content here]]<br />
<br />
===Persistent - The Anti pattern===<br />
#Author Abbas Naderi<br />
# [[CRV2_RevCodePersistentAntiPatternIntro|Put content here]]<br />
<br />
====.NET====<br />
#Author Johanna Curiel, Renchie Joan, Larry Conklin<br />
# [[CRV2_RevCodePersistentAntiPatterndotNet|Put content here]]<br />
<br />
====.Java====<br />
#Author Palak Gohil<br />
# [[CRV2_RevCodePersistentAntiPatternJava|Put content here]]<br />
<br />
====PHP====<br />
#Author Mohammed Damavandi, Abbas Naderi<br />
# [[CRV2_RevCodePersistentAntiPatternPHP|Put content here]]<br />
<br />
====Ruby====<br />
#Author Chris Berberich<br />
# [[CRV2_RevCodePersistentAntiPatternRuby|Put content here]]<br />
<br />
===Reflected - The Anti pattern===<br />
# [[CRV2_RevCodeReflectedAntiPatternIntro|Put content here]]<br />
<br />
====.NET====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_RevCodeReflectedAntiPatterndotNet|Put content here]]<br />
<br />
====.Java====<br />
#Author Palak Gohil<br />
# [[CRV2_RevCodeReflectedAntiPatternJava|Put content here]]<br />
<br />
====PHP====<br />
#Author Mohammed Damavandi, Abbas Naderi<br />
# [[CRV2_RevCodeReflectedAntiPatternPHP|Put content here]]<br />
<br />
====Ruby====<br />
# Author - Open<br />
# [[CRV2_RevCodeReflectedAntiPatternIRuby|Put content here]]<br />
<br />
===Stored - The Anti pattern===<br />
# Author - Open<br />
# [[CRV2_RevCodeStoredAntiPatternIntro|Put content here]]<br />
<br />
====.NET====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_RevCodeStoredAntiPatterndotNET|Put content here]]<br />
<br />
====.Java====<br />
#Author Palak Gohil<br />
# [[CRV2_RevCodeStoredAntiPatternJava|Put content here]]<br />
<br />
====PHP====<br />
#Author Mohammed Damavandi, Abbas Naderi<br />
# [[CRV2_RevCodeStoredAntiPatternPHP|Put content here]]<br />
<br />
====Ruby====<br />
#Author - Open<br />
# [[CRV2_RevCodeStoredAntiPatternRuby|Put content here]]<br />
<br />
===DOM XSS ===<br />
#Author Larry Conklin<br />
# [[CRV2_DOMXSS|Put content here]]<br />
<br />
===JQuery mistakes===<br />
#Author Shenal Silva<br />
# [[CRV2_JQueryMistakes|Put content here]]<br />
<br />
===Reviewing code for SQL Injection===<br />
#Author Palak Gohil, Renchie Joan<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Reviewing_Code_for_SQL_Injection]]<br />
# [[CRV2_RevCodeSQLInjection|Put content here]]<br />
<br />
====PHP====<br />
#Author - Mennouchi Islam Azeddine<br />
# [[CRV2_SQLInjPHP|Put content here]]<br />
<br />
====Java====<br />
#Author - Open<br />
# [[CRV2_SQLInjJava|Put content here]]<br />
<br />
====.NET====<br />
#Author - Mennouchi Islam Azeddine<br />
# [[CRV2_SQLInjdotNET|Put content here]]<br />
<br />
====HQL====<br />
#Author - Open<br />
# [[CRV2_SQLInjHQL|Put content here]]<br />
<br />
===The Anti pattern===<br />
#Author Larry Conklin<br />
#[[CRV2_AntiPattern| Content here]]<br />
https://www.owasp.org/index.php/CRV2_AntiPattern<br />
====PHP====<br />
#Author - Mohammad Damavandi, Abbas Naderi<br />
# [[CRV2_AntiPatternPHP|Put content here]]<br />
<br />
====Java====<br />
#Author - Palak Gohil<br />
#=> Searching for traditional SQL,JPA,JPSQL,Criteria,...<br />
# [[CRV2_AntiPatternJava|Put content here]]<br />
<br />
====.NET====<br />
#Author Johanna Curiel, Renchie Joan,Larry Conklin<br />
# [[CRV2_AntiPatterndotNet|Put content here]]<br />
<br />
====Ruby====<br />
#Author - Open<br />
# [[CRV2_AntiPatternRuby|Put content here]]<br />
<br />
====Cold Fusion====<br />
#Author - Open<br />
# [[CRV2_AntiPatternColdFusion|Put content here]]<br />
<br />
===Reviewing code for CSRF Issues===<br />
#Author Palak Gohil,Anand Prakash, Abbas Naderi<br />
# Previous version to be updated: [[https://www.owasp.org/index.php/Reviewing_Code_for_Cross-Site_Request_Forgery]]<br />
# [[CRV2_CSRFIssues|Put content here]]<br />
<br />
===Transactional logic / Non idempotent functions / State Changing Functions===<br />
#Author Abbas Naderi<br />
# [[CRV2_TransLogic|Put content here]]<br />
<br />
===Reviewing code for poor logic /Business logic/Complex authorization===<br />
#Author - Sam Denard<br />
# [[CRV2_PoorLogic|Put content here]]<br />
<br />
===Reviewing Secure Communications===<br />
====.NET Config====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_SecCommsdotNet|Put content here]]<br />
<br />
====Spring Config====<br />
#Author - Open<br />
# [[CRV2_SecCommsSpringConfig|Put content here]]<br />
<br />
====HTTP Headers====<br />
#Author Gregory Disney, Abbas Naderi<br />
# [[CRV2_SecCommsHTTPHdrs|Put content here]]<br />
<br />
=====CSP=====<br />
#Author Gregory Disney<br />
# [[CRV2_SecCommsHTTPHdrsCSP|Put content here]]<br />
<br />
=====HSTS=====<br />
#Author Abbas Naderi<br />
# [[CRV2_SecCommsHTTPHSTS|Put content here]]<br />
<br />
===Tech-Stack pitfalls===<br />
#Author Gregory Disney<br />
# [[CRV2_TechStackPitfalls|Put content here]]<br />
<br />
===Framework specific Issues===<br />
====Spring====<br />
#Author - Open<br />
# [[CRV2_FrameworkSpecIssuesSpring|Put content here]]<br />
<br />
====Structs====<br />
#Author - Open<br />
# [[CRV2_FrameworkSpecIssuesStructs|Put content here]]<br />
<br />
====Drupal====<br />
#Author Gregory Disney<br />
# [[CRV2_FrameworkSpecIssuesDurpal|Put content here]]<br />
<br />
====Ruby on Rails====<br />
#Author - Open<br />
# [[CRV2_FrameworkSpecIssuesROR|Put content here]]<br />
<br />
====Django====<br />
#Author Gregory Disney<br />
# [[CRV2_FrameworkSpecIssuesDjango|Put content here]]<br />
<br />
====.NET Security / MVC====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_FrameworkSpecIssuesdotNetMVC|Put content here]]<br />
<br />
====Security in ASP.NET applications====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_FrameworkSpecIssuesASPNet|Put content here]]<br />
<br />
=====Strongly Named Assemblies=====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_FrameworkSpecIssuesASPNetStrongAssembiles|Put content here]]<br />
<br />
======Round Tripping======<br />
# Author - Open<br />
# [[CRV2_FrameworkSpecIssuesASPNetRT|Put content here]]<br />
<br />
======How to prevent Round tripping======<br />
# Author - Open<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_FrameworkSpecIssuesASPNetRTPrevention|Put content here]]<br />
<br />
=====Setting the right Configurations=====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_FrameworkSpecIssuesASPNetConfigs|Put content here]]<br />
<br />
=====Authentication Options=====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_FrameworkSpecIssuesASPNetAuth|Put content here]]<br />
<br />
=====Code Review for Managed Code - .Net 1.0 and up=====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_FrameworkSpecIssuesASPNetManagedCode|Put content here]]<br />
<br />
=====Using OWASP Top 10 as your guideline=====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_FrameworkSpecIssuesASPTop10|Put content here]]<br />
<br />
=====Code review for Unsafe Code (C#)=====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_FrameworkSpecIssuesASPNetUnsafeCode|Put content here]]<br />
<br />
====PHP Specific Issues====<br />
#Author Mohammad Damavandi, Abbas Naderi<br />
# [[CRV2_FrameworkSpecIssuesPHP|Put content here]]<br />
<br />
====Classic ASP====<br />
#Author Johanna Curiel<br />
# [[CRV2_FrameworkSpecIssuesASPClassic|Put content here]]<br />
<br />
====C#====<br />
#Author Johanna Curiel, Renchie Joan<br />
# [[CRV2_FrameworkSpecIssuesCsharp|Put content here]]<br />
<br />
====C/C++====<br />
#Author Gary Robinson<br />
# [[CRV2_FrameworkSpecIssuesCplusplus|Put content here]]<br />
<br />
====Objective C====<br />
#Author Open<br />
# [[CRV2_FrameworkSpecIssuesObectiveC|Put content here]]<br />
<br />
====Java====<br />
#Author Palak Gohil<br />
# [[CRV2_FrameworkSpecIssuesJava|Put content here]]<br />
<br />
====Android====<br />
#Author Open<br />
# [[CRV2_FrameworkSpecIssuesAndroid|Put content here]]<br />
<br />
====Coldfusion====<br />
#Author Open<br />
# [[CRV2_FrameworkSpecIssuesColdfusion|Put content here]]<br />
<br />
=Security code review for Agile development=<br />
#Author Carlos Pantelides<br />
# [[CRV2_CodeReviewAgile|Put content here]]<br />
<br />
=Willing to review drafts=<br />
#Terry Nerpester<br />
#Larry Conklin<br />
#Gary Robinson<br />
#Simon Whittaker<br />
#Jason Johnson<br />
#Carlos Pantelides</div>Carlos Pantelides