aiden
/
suricata
mirror of https://github.com/OISF/suricata
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

pppoe: fix protocol field length variation

Browse Source 
Detect when protocol field is not a 16 bit field.
Added tests to prove logic

Ticket: 4810
pull/6802/head
Steven Ottenhoff 3 years ago committed by Victor Julien
parent 260bc03603
commit 6bf2117056
2 changed files with 154 additions and 18 deletions
Show Stats Download Patch File Download Diff File

169
src/decode-pppoe.c
Unescape Escape View File

@ -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 */
}

3
src/decode-pppoe.h
Unescape Escape View File

@ -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)

Write Preview
Loading…
Cancel
Save