xorl %eax, %eax

OpenBSD Remote IP/ICMP Kernel Panic

with 5 comments

Well, I saw this on my RSS feeds before a few minutes and I was kind of surprised that they classified it as a “Reliability Fix“. It’s public knowledge that pointer dereferences can be exploited and even if this isn’t exploitable it’s still a remote kernel panic! Anyway, skipping my comments and moving to the actual bug. You can find it under sys/net/pf.c where PF (Packet Filtering) code is located. Oh.. this definitely affects OpenBSD 4.5 but most likely it’ll also affect earlier releases (I haven’t tested it). There it is:

5638 #ifdef INET
5639 int
5640 pf_test(int dir, struct ifnet *ifp, struct mbuf **m0,
5641     struct ether_header *eh)
    ...
5722         switch (h->ip_p) {
5723 
5724         case IPPROTO_TCP: {
    ...
5754         case IPPROTO_UDP: {
    ...
5784         case IPPROTO_ICMP: {
    ...
5808         default:
5809                 action = pf_test_state_other(&s, dir, kif, m, &pd);
5810                 if (action == PF_PASS) {
    ...
5941         return (action);
5942 }
5943 #endif /* INET */


The above code is (of course) from OpenBSD 4.5 and it is used to determine how it should handle the IPv4 packet received. You can see it checks it against a few protocols or as a last resort it passes it to pf_test_state_other() at line 5809. This function attempts to identify the protocol of the packet but it can easily fall into a KASSERT() which will result in a nice kernel panic since KASSERT() is a wrapper around __assert() which is also a wrapper around panic(). This could be easily triggered if an IPv4 packet has ICMPv6 payload. In a system without INET6 enabled this will almost definitely fall into one of the KASSERT() calls of pf_test_state_other(). On the other hand, if the system has INET6 then the opposite is also possible:

5945 #ifdef INET6
5946 int
5947 pf_test6(int dir, struct ifnet *ifp, struct mbuf **m0,
5948     struct ether_header *eh)
5949 {
    ...
6028                 switch (pd.proto) {
6029                 case IPPROTO_FRAGMENT:
    ...
6035                 case IPPROTO_ROUTING: {
    ...
6065                 case IPPROTO_AH:
6066                 case IPPROTO_HOPOPTS:
6067                 case IPPROTO_DSTOPTS: {
6068                         /* get next header and header length */
    ...
6093         /* if there's no routing header, use unmodified mbuf for checksumming */
    ...
6097         switch (pd.proto) {
6098 
6099         case IPPROTO_TCP: {
    ...
6127         case IPPROTO_UDP: {
    ...
6157         case IPPROTO_ICMPV6: {
    ...
6181         default:
6182                 action = pf_test_state_other(&s, dir, kif, m, &pd);
    ...
6313         return (action);
6314 }
6315 #endif /* INET6 */


The fix to those two bugs was to add another case on the switch statements to check for INET6 (specifically ICMPv6) payload on IPv4 packets and ICMPv4 payload on IPv6 packets respectively:

 	}

+#ifdef INET6
+	case IPPROTO_ICMPV6: {
+		action = PF_DROP;
+		DPFPRINTF(PF_DEBUG_MISC,
+		    ("pf: dropping IPv4 packet with ICMPv6 payload\n"));
+		goto done;
+	}
+#endif
+
 	default:


And for IPv6 function:

 		break;
+	}
+
+	case IPPROTO_ICMP: {
+		action = PF_DROP;
+		DPFPRINTF(PF_DEBUG_MISC,
+		    ("pf: dropping IPv6 packet with ICMPv4 payload\n"));
+		goto done;
 	}

 	case IPPROTO_ICMPV6: {


Now, I might be wrong but do you think this is a reliability bug that has no security impact? I think it does… In addition, OpenBSD claims that this behavior results in a NULL pointer dereference. This sounds like a serious security bug to me.

Written by xorl

April 12, 2009 at 00:59

Posted in bugs, openbsd

5 Responses

Subscribe to comments with RSS.

  1. If it has no security impact, then why send the patch announcement to the security-announce mailing list?

    http://marc.info/?m=123949585205081

    Steve Shockley

    April 13, 2009 at 00:31

  2. That’s really interesting, they initially identify this bug as a security vulnerability and then decide that it’s just a reliability fix…

    xorl

    April 13, 2009 at 09:14

  3. “Only three remote holes in the default install, in more than 10 years!”

    Mo

    April 13, 2009 at 19:53

  4. @Mo: I don’t believe pf is enabled in the default install.

    kamper

    April 18, 2009 at 05:20

  5. that’s right.. it aint enabled by default…

    the realy fucked up fact is, that they did not communicated. other projects do use PF as well and where affected too.

    MirBSD, MidnightBSD, NetBSD 5.x

    All patched, NetBSD before 5.x was released as stable release. :-)

    Btw: Nice and correct analyse! :-)

    rembrandt

    May 5, 2009 at 23:17


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s