[Owasp-testing] [Owasp-codereview] Fwd: Code review Structure

Eoin eoin.keary at owasp.org
Thu Jan 18 08:18:39 EST 2007


Totally agree. Best practice should cover the Top 5 in many languages. - see
wiki structure.
The vulnerability section should include examples of bad code (anti-pattern)
and examples of good code and reasons why for each.
-ek


On 17/01/07, Mark Roxberry <mark.roxberry at mpi.us.com> wrote:
>
>  IMHO, I think if the guide is to have any worth, we need to be specific -
> general recommendations make a good opening section - but code quality is
> ultimately at the level of the code and the proper usage of the language in
> which that code was written.   I think, otherwise, we are talking about the
> items in a general sense that Jim outlined - it's good to know and reiterate
> "Input Validation" as a general term and broken access control examples in
> general terms, but I think it would also be helpful to include best practice
> recommendations.  For example, C++ code can use the strsafe.h functions
> (instead of the early more vulnerable functions, e.g. strcopy) or if using
> VS.NET <http://vs.net/> use /clr:safe, you can compile with many
> restrictive security checks in place or whatever other components /
> libraries specific to the development environment.  I could list out many
> specific practices for each version of ASP.NET <http://asp.net/> (C#,
> VB.NET <http://vb.net/>, C++) / SQL Server, etc.  Another that comes to
> mind - in ADO.NET <http://ado.net/> are we using datareaders and never
> closing the database connection, exposing the system to a DoS, even
> unintentionally?
>
> I'm sorry to ramble, but I think it is important to maybe hit the Top 5
> for each language / environment.  Just my .02.
>
> Mark
>
> ----- Original Message -----
> *From:* Jim Manico <jim at manico.net>
> *To:* Andrew van der Stock <vanderaj at owasp.org>
>  *Cc:* 'Mark Roxberry' <me at markroxberry.net> ;
> Owasp-codereview at lists.owasp.org ; owasp-testing at lists.owasp.org ; 'Eoin'<eoin.keary at owasp.org>
> *Sent:* Wednesday, January 17, 2007 3:50 PM
> *Subject:* Re: [Owasp-codereview] [Owasp-testing] Fwd: Code review
> Structure
>
>
> Noting that the LDAP server is unencrypted is part of a application
> infrastructure audit. Many very smart orginizations do this since encrypting
> LDAP is a HUUUUUGE hit, so they put LDAP in a private subnet and do a few
> other things to not have to encrypt LDAP.
>
> 99% of the time during an audit, there are no requirements docs to match
> against code. So as part of my audit, I both build the proper documentation
> for the app, and write the remediation/audit document. How can you audit
> business processes if there are not requirement's to match them against?
>
> When I do a code audit, I look for 3 main things: (And I'm doing mostly
> J2EE)
>
> 1) Input Validation - like a religion, I check all input data from users,
> config files, databases even (output sanitization), headers - every piece of
> data entering a software system needs to be centrally validated.
> 2) Authentication/Access control
> 3) Session Management (when its custom)
> 4) Overall Code Quality
>
> Then I run all code through Fortify or FindBugs and use that to find
> low-hanging fruit.
>
> Manual review is clocking about 100 lines of code/hr when done with
> integrity, so most of my code reviews involve me creating large teams of
> auditors.
>
> Management always disagrees with me some, so I just document my opinions
> and findings CLEARLY to protect myself, and let management make the call.
>
> - Jim
>
> Andrew van der Stock wrote:
>
> This is actually an important question which needs answering.
>
> Personally, I code review in terms of business function as this is the
> easiest way to demonstrate your value to your client. For example, if I tell
> them that their LDAP server connection is unencrypted, so what? I instead
> look at the use cases from the business requirements and ensure that they
> are properly demonstrated in the code, and if as a side bar to that process,
> I discover weaknesses, I will drop them in, so that:
>
> * The user login process may allow attackers to view all credentials and
> thus log in as anyone they want, including the LDAP manager. This destroys
> any credibility of logs, transactions and may allow widespread destruction
> or alteration of data.
>
> Now the business has a reason to fix my finding as it¹s related to a
> business process and asset they care about.
>
> This is why I¹m not convinced its necessary to have language specific
> chapters. Most of the information in those chapters applies will apply to
> all languages / frameworks. You can always jump over examples you¹re not
> interested in, or you can learn from the other language¹s issues to avoid
> them in your own.
>
> Thanks,
> Andrew
>
>
> On 1/12/07 10:25 AM, "James Kist" <kist at meridiansecurity.net> <kist at meridiansecurity.net> wrote:
>
>
>
> What is the desired structure for the best practices section? How about
> something like this:
>
> Vulnerability (with a link to the section that describes the vulnerability)
> Best practice 1 - Description (includes how and to what level the
> vulnerability is addressed by this best practice)
> Best practice 1 - Code example (if applicable)
> Best practice 2 - Description
> Best practice 2 - Code example (if applicable)
>
> etc.
>
>
>
>
> From: owasp-testing-bounces at lists.owasp.org
> [mailto:owasp-testing-bounces at lists.owasp.org <owasp-testing-bounces at lists.owasp.org>] On Behalf Of Eoin
> Sent: Thursday, January 11, 2007 10:45 AM
> To: Mark Roxberry
> Cc: Owasp-codereview at lists.owasp.org; owasp-testing at lists.owasp.org
> Subject: Re: [Owasp-testing] Fwd: Code review Structure
>
> Hi Mark,
> i believe there is a design section but it has not been touched yet:
>  http://www.owasp.org/index.php/OWASP_Code_Review_Guide_Table_of_Contents
>
> Designing for security (section).
>
> You you consider putting a .NET design section within this.
>
> Authoring the .NET best practice section would be great!! I'll put your name
> beside it.
> thanks,
> Eoin
>
>
> On 11/01/07, Mark Roxberry <me at markroxberry.net> <me at markroxberry.net> wrote:
>
>
>
>
>
> I'll do .NET Code Review Best Practices.
>
>
>
> Can I include Design Guidance as a section?  Or maybe we need to  consider
> Secure Application Design for an OWASP project (or do we have plans  for this
> already)?  An example, in ASP.NET <http://asp.net/> <http://asp.net/> <http://asp.net/>  2.0, when do we
> recommend using the  MembershipProviders and integrating with .NET framework
> before rolling your  own access control system.  Design guidance would
> outline the  scenarios for each security design.  What do you think?
>
>
>
> I'll post a topic list by tomorrow.
>
>
> Regards,
>
>
>
> Mark
>
>
>
>
>
> ----- Original Message -----
>
> From: Eoin <mailto:eoin.keary at owasp.org> <eoin.keary at owasp.org>
>
> To: owasp-testing at lists.owasp.org  <mailto:owasp-testing at lists.owasp.org> <owasp-testing at lists.owasp.org> ;Owasp-codereview at lists.owasp.org
>
> Sent: Tuesday, January 09, 2007 10:47  AM
>
> Subject: [Owasp-testing] Fwd: Code  review Structure
>
>
>
>
> Hi,
>
> Below is the current structure of the code review guide.
>
>
> If anyone would like to take on a section (improve a section/add  more info)
> please let me know and ill pen you in for it.
>
> thanks,
>
> Eoin
>
>
>
>
>
> Methodology
>
>
> Introduction
>
> Steps and  Roles
>
> Code Review  Processes
>
>
> Design review
> Designing for security
>
>
> Examples by Vulnerability
>
>
>
>
>
>
>
>
>
>         Buffer Overruns and  Overflows
>
> OS Injection
>
> SQL Injection
>
> Data  Validation
>
> Error Handling
>
> Logging issues
>
> The Secure Code  Environment
>
> Transaction  Analysis
>
> Authorization
>
> Authentication
>
> Session  Integrity
>
> Cross Site Request  Forgery
>
> Cryptography
>
> Dangerous HTTP  Methods
>
> Race  Conditions
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> Language specific best practice
>
> Java
>
>
> Inner  classes
>
> Class  comparison
>
> Cloneable  classes
>
> Serializable  classes
>
> Package scope and  encapsulation
>
> Mutable  objects
>
> Private methods &  circumvention
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> .NET
>
> PHP
>
> Automating Code Reviews
>
>
> Preface
>
> Reasons for using automated  tools
>
> Education and cultural  change
>
> Tool Deployment  Model
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> References
>
>
>
>
>
>   ------------------------------
>
> _______________________________________________
> Owasp-codereview mailing listOwasp-codereview at lists.owasp.orghttp://lists.owasp.org/mailman/listinfo/owasp-codereview
>
>
> --
> Best Regards,
> Jim Manico
> GIAC GSEC Professional, Sun Certified Java Programmerjim at manico.net808.652.3805
>
>  ------------------------------
>
> _______________________________________________
> Owasp-codereview mailing list
> Owasp-codereview at lists.owasp.org
> http://lists.owasp.org/mailman/listinfo/owasp-codereview
>
>
> _______________________________________________
> Owasp-testing mailing list
> Owasp-testing at lists.owasp.org
> http://lists.owasp.org/mailman/listinfo/owasp-testing
>
>
>


-- 
Eoin Keary OWASP - Ireland
http://www.owasp.org/local/ireland.html
http://www.owasp.org/index.php/OWASP_Testing_Project
http://www.owasp.org/index.php/OWASP_Code_Review_Project
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.owasp.org/pipermail/owasp-testing/attachments/20070118/e8879372/attachment-0001.html 


More information about the Owasp-testing mailing list