Background For This Talk

1. The Power of Code ReviewIf you aren’t doing it, you really shouldDave WichersCOO, Aspect SecurityOWASP Board MemberOWASP Top 10 and ASVS Projects LeadOWASP Conferences Chairdave.wichers@aspectsecurity.comdave.wichers@owasp.org

2. Background For This Talk First off, I’m talking about • Manual Code Review vs. Manual Pen Testing • Both are tool assisted, of course. Both techniques can use/leverage automated analysis tools • But I don’t think that affects my argument

3. Why Do We Look For Vulnerabilities? • To find them? • To find exactly where they are in the app? • To verify we don’t have them? Which technique is best suited to answer these questions? • Application Penetration Testing, or • Code Review

4. What do I Mean by Code Review vs. Pen Testing • Code Review should review all • custom developed code, AND • config files of the app, libraries, frameworks, app server, etc. • App Pen Testing • Tests the application from the outside • If you can get access to config files, you should of course review them too • Ideally you are doing both! • But let’s compare their strengths and weaknesses

5. Both Have Their Advantages • Pen Testing Pros • Requires less specialized expertise • Easier setup • Easier to perform • Exercises the entire app infrastructure • Proves vulnerabilities • Code Review Pros - Easier to • Find all the content • Find all instances of certain types of flaws • Verify controls are correct • Verify controls are used in all the required places

6. How do they stack up to the OWASP Top 10? http://www.owasp.org/index.php/Top_10

7. Finding Injection Flaws – Testing? ' ; droptable log -- Tool assistance is very helpful here.

8. Finding Injection Flaws – Reviewing Code? Then look for dangerous concatenation. try { String query = "SELECT employee.* FROM employee WHERE employer_name = '" + username + "' and employee_name = '" + subjectUsername + "'"; Statement answer_statement = session.createStatement(…); ResultSetanswer_results = answer_statement.executeQuery(query); … } catch (SQLExceptionsqle) { … • First, search for things like: • exec • SQL • J/ODBC try { String query = "SELECT employee.* FROM employee WHERE employer_name = ? and employee_name = ?"; PreparedStatement answer = session.prepareStatement( query, …); answer.setString( 1, username ); answer.setString( 2, subjectUsername ); ResultSetanswer_results = answer.executeQuery(); … Or safe use of Bind Variables.

9. Injection Results Key: Ratings on a Scale from 1 (hardest) to 5 (easiest) Check out the: http://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet for how to avoid SQL injection vulnerabilities.

10. Finding XSS Flaws – Testing? <script>alert(document.cookie)</script> <script>alert(document.cookie)</script> <script>alert(document.cookie)</script> or ?

11. Finding XSS Flaws – Reviewing Code? Or output encoding to make inclusion of user content safe. (Imagine this .jsp) <Table> <TR><TD> Full Name: </TD> <TD> <%=user.getFirstName()%> <%=user.getLastName()%> </TD> <TD> Display Name: </TD> <TD> <%=user.getDisplayName()%> </TD> <Table> <TR><TD> Full Name: </TD> <TD> <%=ESAPI.encodeForHTML(user.getFirstName())%> <%= ESAPI.encodeForHTML( user.getLastName())%> </TD> <TD> Display Name: </TD> <TD> <%= ESAPI.encodeForHTML( user.getDisplayName())%> </TD> Look for unsafe user content inclusion.

12. XSS Results Check out the: http://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet for how to avoid XSS vulnerabilities.

13. Finding AuthN & Session Mgt Flaws – Testing? sometimes its obvious http://www.stealmycustomersmoney.com/onlineStatement.do;jsessionid=7371FC8C8C4C228DECEDB0FCE5818789 and lots of testing is easy Well …

14. Finding AuthN & Session Mgt Flaws – Code Review? <security-constraint>    <web-resource-collection>       <web-resource-name>User data</web-resource-name>       <url-pattern>/banking/user/*</url-pattern>    </web-resource-collection>    <auth-constraint>       <role-name>User</role-name>    </auth-constraint> </security-constraint> … Can you figure out all of the resources that require authentication and which don’t? Can you tell if these hashes are properly salted? bsmith:1045:Ejrt3EJUnh5Ms:Bob:Smith dwilson:1046:1MvGJq5Nqersjw:Dave:Wilson dwichers:1243:4jaM4lehUwp25N:Dave:Wichers Server=172.145.34.2;Database=pubs;UserID=sa;Password=stealme" Can you find all the stored credentials to make sure they are encrypted? But sometimes its hard

15. Common AuthN & Session MgtReqts* ? * - Requirements selected from OWASP ASVS

16. Authentication and Session Mgt Results ? ? Check out the: http://www.owasp.org/index.php/Authentication_Cheat_Sheet for how to avoid authentication vulnerabilities.

17. Finding Direct Object Reference Flaws –Testing? https://www.onlinebank.com/user?acct=6065

18. Finding Direct Object Reference Flaws –Code Review? • String query = "SELECT account_balance FROM accounts WHEREaccount_number = ?"; • PreparedStatementpstmt = connection.prepareStatement( query, … ); • pstmt.setInt( 1, account); Is account authorized for this user? Maybe it was validated earlier. • String query = "SELECT account_balance FROM accounts WHEREaccount_number = ? and account_owner = ?"; • PreparedStatementpstmt = connection.prepareStatement( query, … ); • pstmt.setInt( 1, account); • pstmt.setInt( 2, currentUser ); This time its clear that only the owner can retrieve this data. File fileToRetrieve = new File(usersuppliedfilename)); Was this filename validated? Is the user authorized for this file? @Secured public Plan retrievePlan(PlanIdpidId) { … } @Secured invokes an authorization check that ensures the user is authorized for this plan. Is this annotation on every method that requires this protection? Its easy to check the code.

19. Direct Object Reference Results

20. Finding CSRF Flaws – Testing? POST BODY: givenName=David& familyName=Wichers& nickname=DavidW& gender=M& crumb=B4k22G2EReX crumb=g4bxSkzUEY2  Same form after logout and log back in. http://www.somebank.com/atmwithdraw?acct=1756403&amt=300&csrf=h4Ul43xQ20 CSRF Testing is pretty darn easy. But if there are LOTS of pages and functions, it can be tedious to find and test them all.

21. Finding CSRF Flaws – Code Review? public void doFilter(ServletRequestrequest, ServletResponseresponse, FilterChain chain) throws IOException, ServletException{ booleanrequestAllowed = false; // Code is here to verify request requires CSRF Check // If not, set requestAllowed = true; // If so, check the following: String csrfReqToken = request.getParameter("tokenname"); String userCSRFToken = (String) request.getSession().get("userCSRFtoken"); if (userCSRFToken.equals(csrfReqToken)) requestAllowed = true; However they verify them, they are pretty easy to check in code too. However, in code, if this is checked in a chokepoint, like a filter, then its very easy to verify the entire app quickly, rather than having to check EVERY page on the site.

22. CSRF Results Check out the: http://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet for how to avoid CSRF vulnerabilities.

23. Finding Config Flaws – Testing? Config, to me, involves configuration of OS, Web Server, App Server, DB Server, frameworks, etc. If testers have access to config, then advantage is clearly with testers, but they usually don’t. Code reviewers frequently don’t either actually. Certainly not all of it. Automated testing is frequently the most effective technique for detecting most product level config flaws. Reviewing the actual configuration is extremely helpful too.

24. Finding Config Flaws – Code Review? Gathering all the config information for a reviewer is typically challenging. Understanding what all the different configuration information means is also challenging. Reviewing config is very effective when you understand what it all means. But clearly, to me, advantage is with the testers here.

25. Config Results

26. Finding URL Based Access Control Flaws – Testing? Is each of these protected from unauthorized access? Force access from other user roles to verify. Pretty easy to test. But: Even if 1 role is prevented, are ALL unauthorized roles prevented? To me this top 10 category is function level access control, not just URLs. http://www.thebank.com/admin/getsitestats http://www.thebank.com/bankemployee/getacctbalance?cust=129843 http://www.thebank.com/bankemployee/main?button=deletecust&cust=23489 http://www.thebank.com/admin/main POST BODY: button=setuserpwd& cust=84375& pwd1=ThEsecret1& pwd2=ThEsecret1& Token=m7G34lkw9Xr

27. Finding URL Based Access Control Flaws – Code Review? ? Checking a single function is about the same with code review. Finding whether the controls are in place everywhere they are needed, and if they are correct is MUCH easier with code review. @Secured public intgetAccountBalance(intcustomerId) throws ServiceException { … public void transferFunds(intsrcAcct, intdestAcct, int amount) throws ServiceException { … @Secured public intcreateNewAccount(intcustomerId) throws ServiceException { … - - - - - - - - - App 2 - - - - - - - public void addUser(String username, intcustomerId) throws ServiceException { if (isAuthorized((Integer) session.get("currentUser")) {

28. URL Based Access Control Results

29. Finding Crypto Storage Flaws – Testing? • Practically all crypto storage flaws are very difficult to test for as they are generally invisible to the user • Algorithm selected • Strength of key • Key protection • Is data encrypted? • Etc. • Not much can be done with straight pen testing • Unless you can break into the server  • But, you may be able to get the configuration files or even DB. If so, most of this can be reviewed without looking at the code.

30. Finding Crypto Storage Flaws – Code Review? ? • Typically very straightforward with code review • And you typically also have access to the config * - Requirements selected from OWASP ASVS

31. Crypto Storage Results Check out the: http://www.owasp.org/index.php/Cryptographic_Storage_Cheat_Sheet for how to avoid cryptographic storage vulnerabilities.

32. Finding Transport Security Flaws – Testing? • Most items are easier to find via testing • Is SSL/TLS used for all network connections transmitting sensitive data? • Is the Server cert valid for this domain, not expired, not revoked, etc.? • Does the server support weak protocols? • Does the server support weak ciphers?

33. Finding Transport Security Flaws – Code Review? • But some may be easier via code review • Verifying back end connections are also properly secured. • Verifying that all ‘client’ side SSL connections generated by the application perform all the required certificate verification checks (e.g., trusted, valid, expired, revoked). • And back end server side connections too. • Verifying that any such SSL negotiation failures are properly logged. Note: If this is all done by products or libraries then this is just hard!

34. Transport Security Results See www.ssllabs.com for a battery of free and commercial SSL automated tests. And the: http://www.owasp.org/index.php/Transport_Layer_Protection_Cheat_Sheet for how to avoid transport layer security vulnerabilities.

35. Finding Redirect/Forward Flaws – Testing? http://www.irs.gov/taxrefund/claim.jsp?year=2006&… &dest=www.evilsite.com http://www.irs.gov/taxrefund/checkValid.jsp POST BODY: year=2007& SSN=534684375& lastname=wichers& function=/taxrefund/approved.jsp Finding the Redirect / Forward parameters can be a pain. Testing redirect parameters is really EASY. Testing for an unvalidated forward can be HARD.

36. Finding Redirect/Forward Flaws – Code Review? Was target validated? Finding redirects and forward in code is EASY. Verifying they are safe or not is pretty easy too. public void doPost( HttpServletRequest request, HttpServletResponse response) { try { String target = request.getParameter( "dest" ) ); ... request.getRequestDispatcher( target ).forward(request, response); } catch ( ... ... response.sendRedirect( target ); ...

37. Redirect/Forward Results

38. Results Summary

39. Revisiting Our Initial Questions • To find them? • Result: Similar, but slight edge to code review. • It’s a MYTH that code review is way more expensive. If you have people with the right skills, its actually faster AND more effective. • To find exactly where they are in the app? • Result: Clear advantage to code review • To verify we don’t have them? • Result: Also clearly the advantage goes to code review

40. Some Success Stories • Large Financial Organization Working to Stamp Out XSS (30M lines of code) • Manual code review to find XSS flaws • Regex patterns developed to find similar flaws • Patterns run and validated to find 1000+ XSS flaws in subset of this codebase in 2 weeks • Large ISP • Manual code review to find XSS and SQL Injection • Hundred+ of flaws found and fixed in 2 man weeks

41. Conclusion The Ideal: Do both, … but sometimes you can’t. You should ALWAYS try. • Knowing where the vulns are in the code allows you to provide much better remediation guidance. • Try to always report where in the code the vulnerability is • And how to fix the code • The advantage of code review grows significantly with • size of the application • size of the application portfolio • the level of rigor of the assessment