Your browser doesn't support the features required by impress.js, so you are presented with a simplified version of this presentation.

For the best experience please use the latest Chrome , Safari or Firefox browser.

Security
Code Review

Krisztián Schäffer, HACKTIVITY 2012
 
Code Review in general
Quality Assurance Activity
Common practice on big projects
"White crow" in the enterprise
Mature tools and processes
Every change is reviewed before commit
 Security Code Review
Research
CVE 2012-2122
MySQL/MariaDB auth bypass
$ for i in `seq 1 1000`;
do mysql -u root
--password=bad
-h 127.0.0.1
2>/dev/null; done
mysql>
typedef char my_bool;

my_bool check_scramble(const char *scramble_arg,
            const char *message,
            const uint8 *hash_stage2)
{
  SHA1_CONTEXT sha1_context;
  uint8 buf[SHA1_HASH_SIZE];
  uint8 hash_stage2_reassured[SHA1_HASH_SIZE];

  .... Hashing stuff ...

  return memcmp(hash_stage2,
               hash_stage2_reassured,
               SHA1_HASH_SIZE);
}
                     
typedef char my_bool;
static my_bool check_pwd(char *given_hash,
                      char *stored_hash)
{
  return memcmp(given_hash, stored_hash, 8);
}
int main()
{
  char *given =  "\x00 xxxxaaa";
  char *stored = "\xff aaaaaaa";
  printf("memcmp: %d\n",
      memcmp(given, stored, 8));
  printf("check : %d\n",
      (int)check_pwd(given, stored));
  return 0;
}
                     
$ gcc -Wall -pedantic
mysqlproba.c
$                    
Clang says nothing, but
Splint 3.1.2
mysqlproba.c: (in function
                     check_pwd)
mysqlproba.c:7:10: Return
value type int does not match
declared type my_bool:
memcmp(given_hash, stored_hash, 8)
Types are incompatible.
                     
- my_bool schould be defined as int.
- But the main problem is the mental transformation between two 'equivalent' expressions:
memcmp(a,b) <=> memcmp(a,b) != 0
This is 'C style', but the problem is universal.
/*
* CVE-2012-2122 checker, Joshua J. Drake
*/
int main(void) {
  ...
  while (1) {
    one = rand();
    two = rand();
    ret = memcmp(&one, &two, sizeof(int));
    if (ret < -128 || ret > 127)
      break;
    ... abort if enough time spent ...
  }
  printf("Vulnerable! memcmp returned: %d\n",
        ret);
}
                     
∃ a,b: memcmp(a,b) < -128
 || memcmp(a,b) > 127
vs.
∃ a,b: (char)memcmp(a,b) == 0
       && memcmp(a,b) != 0
(char)-129 == 127
(char)-130 == 126
(char) 128 == -127
(char) 129 == -126
Why unexplored so long?
Research goal is to find vulnerabilities, not weaknesses.
"There is no such thing as crash or DoS, there are vulns YOU cannot exploit."
who?
Research
Audit
Code Audit vs. b/g box
Trust
Focus
Feedback
Attack simulation
Vulnerability types
Coverage
/tmp/src.zip?
-> Hybridize
Focus on
- Most critical vulns
- Easy to find vulns
- Code coverage
Time/money, business needs matter
Easy to forget it during the work
and run for the interesting vulns.
Threat model will
help focusing
Ask for it or DIY
- Attack surface
- Threats
- Assets
Attack tree, DFD, ...
Methods
- Fagan: Heavyweight
- Timeboxed w or w/o
dev support
- Face to face: ideal
for low budget
- Fully automated: Just like automated hacking
Navigation
- Control flow sensitive
- Data flow sensitive
- Top-down
Starting points
- Input
- Candidate points:
grep, static analyser, security functionality,
trust boundary etc.
- Top of the file
What is a finding?
Vuln: You have to prove
Weakness: Too many?
You as auditor are an
expensive resource so
don't waste your time!
public void serialize(ProtocolObject object,
 Writer output) throws ProtocolException
{
try {
 AuthorizationRequest authorizationRequest = ...
 output.write("<?xml version=\"1.0\" ?>");
 output.write("<authz-request version=\"0.1\">");
 output.write("   <subject>");
 output.write("      <subject-id>"+
            authorizationRequest.getToken()+
            "</subject-id>");
 output.write("   </subject>");
 output.write("   <resource>");
 output.write("      <resource-id>"+
  authorizationRequest.getResource()+
  "</resource-id>");
                     
string key = username +
          DateTime.Now.Ticks.ToString();
SHA512 sha = SHA512.Create();
string authToken = Convert.ToBase64String(
                 sha.ComputeHash(
                  Encoding.UTF8.GetBytes(key)
                 )
                ).Replace("/","");
                     
Audit
Assurance
Regular review is part
of good QA - do it!
Develop security culture
Findig a weakness is a victory, not shame on the developer
Identify, separate and audit high risk components
Use tools to have data and metrics
- Inspection rate (LOC/hour)
- Defect rate (defect/hour)
- Defect density (defect/kLOC)
These tell more about the review than the code!
static analysis
Have a secure coding codex and review checklist per language
Buy one or start with a naked platform codex and tune it in iterations
public static Class<?> findClass(String name) ...
 try {
  ClassLoader loader = Thread.currentThread().
                      getContextClassLoader();
  if (loader == null) {
   // can be null in IE (see 6204697)
   loader = ClassLoader.getSystemClassLoader();
  }
  if (loader != null) {
   return Class.forName(name, false, loader);
  }
 } catch (ClassNotFoundException exception) {
  // use current class loader instead
 } catch (SecurityException exception) {
  // use current class loader instead
 }
 return Class.forName(name);
                 
crypto k.o$ cat README
#@+leo-ver=4-thin
#@+node:xxx.20060802163624:@thin README
This is an experiment to use Literate Programming in the X project. All files in the crypto package are generated from the single crypto.leo file. You are encouraged to edit this file with Leo, instead of editing the derived java files.
...
#@nonl
#@-node:xxx.20060802163624:@thin README
#@-leo
Thank you!
Krisztián Schäffer
ko.co
Made with impress.js
if (orgUnit == null || template == null ||
      !(currentWorker.isOwner() ||
      currentWorker.isSecurity() ||
      currentWorker.isResolver(orgUnit) ||
      currentWorker.isTopResolver() ||
      currentWorker.isLeader()) ||
      rightApplication == null)
  {
    %>
    <%@ include file="/include/err_authz.jsp"%>
    <%
    return;
}