diff --git a/src/decode-pppoe.c b/src/decode-pppoe.c index 757a276211..1030552a85 100644 --- a/src/decode-pppoe.c +++ b/src/decode-pppoe.c @@ -38,7 +38,6 @@ #include "decode-ppp.h" #include "decode-pppoe.h" #include "decode-events.h" - #include "flow.h" #include "util-validate.h" @@ -135,7 +134,7 @@ int DecodePPPOESession(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, StatsIncr(tv, dtv->counter_pppoe); - if (len < PPPOE_SESSION_HEADER_LEN) { + if (len < PPPOE_SESSION_HEADER_MIN_LEN) { ENGINE_SET_INVALID_EVENT(p, PPPOE_PKT_TOO_SMALL); return TM_ECODE_FAILED; } @@ -146,12 +145,31 @@ int DecodePPPOESession(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, PPPOE_SESSION_GET_VERSION(p->pppoesh), PPPOE_SESSION_GET_TYPE(p->pppoesh), p->pppoesh->pppoe_code, SCNtohs(p->pppoesh->session_id), SCNtohs(p->pppoesh->pppoe_length)); /* can't use DecodePPP() here because we only get a single 2-byte word to indicate protocol instead of the full PPP header */ - if (SCNtohs(p->pppoesh->pppoe_length) > 0) { /* decode contained PPP packet */ - switch (SCNtohs(p->pppoesh->protocol)) - { + uint8_t pppoesh_len; + uint16_t ppp_protocol = SCNtohs(p->pppoesh->protocol); + + /* According to RFC1661-2, if the least significant bit of the most significant octet is + * set, we're dealing with a single-octet protocol field */ + if (ppp_protocol & 0x0100) { + /* Single-octet variant */ + ppp_protocol >>= 8; + pppoesh_len = PPPOE_SESSION_HEADER_MIN_LEN; + } else { + /* Double-octet variant; increase the length of the session header accordingly */ + pppoesh_len = PPPOE_SESSION_HEADER_MIN_LEN + 1; + + if (len < pppoesh_len) { + ENGINE_SET_INVALID_EVENT(p, PPPOE_PKT_TOO_SMALL); + return TM_ECODE_FAILED; + } + } + + SCLogDebug("Protocol %" PRIu16 " len %" PRIu8 "", ppp_protocol, pppoesh_len); + + switch (ppp_protocol) { case PPP_VJ_COMP: case PPP_IPX: case PPP_OSI: @@ -185,46 +203,45 @@ int DecodePPPOESession(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, case PPP_VJ_UCOMP: - if(len < (PPPOE_SESSION_HEADER_LEN + IPV4_HEADER_LEN)) { + if (len - pppoesh_len < IPV4_HEADER_LEN) { ENGINE_SET_INVALID_EVENT(p, PPPVJU_PKT_TOO_SMALL); return TM_ECODE_OK; } - if (unlikely(len > PPPOE_SESSION_HEADER_LEN + USHRT_MAX)) { + if (unlikely(len - pppoesh_len > USHRT_MAX)) { return TM_ECODE_FAILED; } - if(IPV4_GET_RAW_VER((IPV4Hdr *)(pkt + PPPOE_SESSION_HEADER_LEN)) == 4) { - DecodeIPV4(tv, dtv, p, pkt + PPPOE_SESSION_HEADER_LEN, len - PPPOE_SESSION_HEADER_LEN); + if (IPV4_GET_RAW_VER((IPV4Hdr *)(pkt + pppoesh_len)) == 4) { + DecodeIPV4(tv, dtv, p, pkt + pppoesh_len, len - pppoesh_len); } break; case PPP_IP: - if(len < (PPPOE_SESSION_HEADER_LEN + IPV4_HEADER_LEN)) { + if (len - pppoesh_len < IPV4_HEADER_LEN) { ENGINE_SET_INVALID_EVENT(p, PPPIPV4_PKT_TOO_SMALL); return TM_ECODE_OK; } - if (unlikely(len > PPPOE_SESSION_HEADER_LEN + USHRT_MAX)) { + if (unlikely(len - pppoesh_len > USHRT_MAX)) { return TM_ECODE_FAILED; } - - DecodeIPV4(tv, dtv, p, pkt + PPPOE_SESSION_HEADER_LEN, len - PPPOE_SESSION_HEADER_LEN); + DecodeIPV4(tv, dtv, p, pkt + pppoesh_len, len - pppoesh_len); break; /* PPP IPv6 was not tested */ case PPP_IPV6: - if(len < (PPPOE_SESSION_HEADER_LEN + IPV6_HEADER_LEN)) { + if (len - pppoesh_len < IPV6_HEADER_LEN) { ENGINE_SET_INVALID_EVENT(p, PPPIPV6_PKT_TOO_SMALL); return TM_ECODE_OK; } - if (unlikely(len > PPPOE_SESSION_HEADER_LEN + USHRT_MAX)) { + if (unlikely(len - pppoesh_len > USHRT_MAX)) { return TM_ECODE_FAILED; } - DecodeIPV6(tv, dtv, p, pkt + PPPOE_SESSION_HEADER_LEN, len - PPPOE_SESSION_HEADER_LEN); + DecodeIPV6(tv, dtv, p, pkt + pppoesh_len, len - pppoesh_len); break; default: - SCLogDebug("unknown PPP protocol: %" PRIx32 "",SCNtohs(p->pppoesh->protocol)); + SCLogDebug("unknown PPP protocol: %" PRIx32 "", ppp_protocol); ENGINE_SET_INVALID_EVENT(p, PPP_WRONG_TYPE); return TM_ECODE_OK; } @@ -447,6 +464,120 @@ static int DecodePPPOEtest06 (void) } PASS; } + +/** DecodePPPOEtest07 + * \brief Valid PPPOE packet with 8 bit protocol field - check the valid ICMP type is accepted + * \retval 1 Expected test value + */ +static int DecodePPPOEtest07(void) +{ + + uint8_t raw_pppoe[] = { 0x11, 0x00, 0x00, 0x2d, 0x00, 0x1c, 0x21, 0x45, 0x00, 0x00, 0x1d, 0x97, + 0xc3, 0x00, 0x00, 0x40, 0x01, 0x47, 0x0f, 0x0a, 0x64, 0x00, 0x00, 0xc0, 0xa8, 0xd1, 0x01, + 0x08, 0x00, 0xd4, 0x4c, 0x1f, 0x32, 0x04, 0x81, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00 }; + + Packet *p = PacketGetFromAlloc(); + if (unlikely(p == NULL)) + FAIL; + ThreadVars tv; + DecodeThreadVars dtv; + + memset(&tv, 0, sizeof(ThreadVars)); + memset(&dtv, 0, sizeof(DecodeThreadVars)); + + DecodePPPOESession(&tv, &dtv, p, raw_pppoe, sizeof(raw_pppoe)); + + FAIL_IF(ENGINE_ISSET_EVENT(p, PPP_WRONG_TYPE)); + SCFree(p); + PASS; +} + +/** DecodePPPOEtest08 + * \brief Valid PPPOE packet with 8 bit protocol field - check the valid HTTP type is accepted + * \retval 1 Expected test value + */ +static int DecodePPPOEtest08(void) +{ + + uint8_t raw_pppoe[] = { 0x11, 0x00, 0x00, 0x2d, 0x00, 0x3d, 0x21, 0x45, 0x00, 0x00, 0x3c, 0x00, + 0x00, 0x40, 0x00, 0x40, 0x06, 0xed, 0xda, 0x0a, 0x64, 0x00, 0x00, 0x8e, 0xfa, 0xb3, 0x83, + 0xde, 0xb5, 0x00, 0x50, 0xd4, 0xbd, 0x76, 0x54, 0x00, 0x00, 0x00, 0x00, 0xa0, 0x02, 0xfe, + 0xcc, 0x74, 0x2f, 0x00, 0x00, 0x02, 0x04, 0x05, 0xac, 0x01, 0x03, 0x03, 0x07, 0x04, 0x02, + 0x08, 0x0a, 0xcb, 0xae, 0x92, 0x63, 0x00, 0x00, 0x00, 0x00 }; + + Packet *p = PacketGetFromAlloc(); + if (unlikely(p == NULL)) + FAIL; + ThreadVars tv; + DecodeThreadVars dtv; + + memset(&tv, 0, sizeof(ThreadVars)); + memset(&dtv, 0, sizeof(DecodeThreadVars)); + + DecodePPPOESession(&tv, &dtv, p, raw_pppoe, sizeof(raw_pppoe)); + + FAIL_IF(ENGINE_ISSET_EVENT(p, PPP_WRONG_TYPE)); + SCFree(p); + PASS; +} + +/** DecodePPPOEtest09 + * \brief Valid PPPOE packet with 16 bit protocol field - check the valid ICMP type is accepted + * \retval 1 Expected test value + */ +static int DecodePPPOEtest09(void) +{ + + uint8_t raw_pppoe[] = { 0x11, 0x00, 0x00, 0x2d, 0x00, 0x1c, 0x00, 0x21, 0x45, 0x00, 0x00, 0x1d, + 0x97, 0xc3, 0x00, 0x00, 0x40, 0x01, 0x47, 0x0f, 0x0a, 0x64, 0x00, 0x00, 0xc0, 0xa8, 0xd1, + 0x01, 0x08, 0x00, 0xd4, 0x4c, 0x1f, 0x32, 0x04, 0x81, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00 }; + + Packet *p = PacketGetFromAlloc(); + if (unlikely(p == NULL)) + FAIL; + ThreadVars tv; + DecodeThreadVars dtv; + + memset(&tv, 0, sizeof(ThreadVars)); + memset(&dtv, 0, sizeof(DecodeThreadVars)); + + DecodePPPOESession(&tv, &dtv, p, raw_pppoe, sizeof(raw_pppoe)); + + FAIL_IF(ENGINE_ISSET_EVENT(p, PPP_WRONG_TYPE)); + SCFree(p); + PASS; +} + +/** DecodePPPOEtest10 + * \brief Valid PPPOE packet with 16 bit protocol field - check the valid HTTP type is accepted + * \retval 1 Expected test value + */ +static int DecodePPPOEtest10(void) +{ + + uint8_t raw_pppoe[] = { 0x11, 0x00, 0x00, 0x2d, 0x00, 0x3d, 0x00, 0x21, 0x45, 0x00, 0x00, 0x3c, + 0x00, 0x00, 0x40, 0x00, 0x40, 0x06, 0xed, 0xda, 0x0a, 0x64, 0x00, 0x00, 0x8e, 0xfa, 0xb3, + 0x83, 0xde, 0xb5, 0x00, 0x50, 0xd4, 0xbd, 0x76, 0x54, 0x00, 0x00, 0x00, 0x00, 0xa0, 0x02, + 0xfe, 0xcc, 0x74, 0x2f, 0x00, 0x00, 0x02, 0x04, 0x05, 0xac, 0x01, 0x03, 0x03, 0x07, 0x04, + 0x02, 0x08, 0x0a, 0xcb, 0xae, 0x92, 0x63, 0x00, 0x00, 0x00, 0x00 }; + + Packet *p = PacketGetFromAlloc(); + if (unlikely(p == NULL)) + FAIL; + ThreadVars tv; + DecodeThreadVars dtv; + + memset(&tv, 0, sizeof(ThreadVars)); + memset(&dtv, 0, sizeof(DecodeThreadVars)); + + DecodePPPOESession(&tv, &dtv, p, raw_pppoe, sizeof(raw_pppoe)); + + FAIL_IF(ENGINE_ISSET_EVENT(p, PPP_WRONG_TYPE)); + SCFree(p); + PASS; +} #endif /* UNITTESTS */ @@ -464,6 +595,10 @@ void DecodePPPOERegisterTests(void) UtRegisterTest("DecodePPPOEtest04", DecodePPPOEtest04); UtRegisterTest("DecodePPPOEtest05", DecodePPPOEtest05); UtRegisterTest("DecodePPPOEtest06", DecodePPPOEtest06); + UtRegisterTest("DecodePPPOEtest07", DecodePPPOEtest07); + UtRegisterTest("DecodePPPOEtest08", DecodePPPOEtest08); + UtRegisterTest("DecodePPPOEtest09", DecodePPPOEtest09); + UtRegisterTest("DecodePPPOEtest10", DecodePPPOEtest10); #endif /* UNITTESTS */ } diff --git a/src/decode-pppoe.h b/src/decode-pppoe.h index 6aecf74151..44550ad112 100644 --- a/src/decode-pppoe.h +++ b/src/decode-pppoe.h @@ -27,7 +27,8 @@ #include "decode.h" #include "threadvars.h" -#define PPPOE_SESSION_HEADER_LEN 8 +// Session header length minus the protocol field +#define PPPOE_SESSION_HEADER_MIN_LEN 7 #define PPPOE_DISCOVERY_HEADER_MIN_LEN 6 #define PPPOE_SESSION_GET_VERSION(hdr) ((hdr)->pppoe_version_type & 0xF0) >> 4 #define PPPOE_SESSION_GET_TYPE(hdr) ((hdr)->pppoe_version_type & 0x0F)