Commit f5eb917b authored by John Hughes's avatar John Hughes Committed by David S. Miller

x25: Patch to fix bug 15678 - x25 accesses fields beyond end of packet.

Here is a patch to stop X.25 examining fields beyond the end of the packet.

For example, when a simple CALL ACCEPTED was received:

	10 10 0f

x25_parse_facilities was attempting to decode the FACILITIES field, but this
packet contains no facilities field.
Signed-off-by: default avatarJohn Hughes <john@calva.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent fd218cf9
...@@ -182,6 +182,10 @@ extern int sysctl_x25_clear_request_timeout; ...@@ -182,6 +182,10 @@ extern int sysctl_x25_clear_request_timeout;
extern int sysctl_x25_ack_holdback_timeout; extern int sysctl_x25_ack_holdback_timeout;
extern int sysctl_x25_forward; extern int sysctl_x25_forward;
extern int x25_parse_address_block(struct sk_buff *skb,
struct x25_address *called_addr,
struct x25_address *calling_addr);
extern int x25_addr_ntoa(unsigned char *, struct x25_address *, extern int x25_addr_ntoa(unsigned char *, struct x25_address *,
struct x25_address *); struct x25_address *);
extern int x25_addr_aton(unsigned char *, struct x25_address *, extern int x25_addr_aton(unsigned char *, struct x25_address *,
......
...@@ -82,6 +82,41 @@ struct compat_x25_subscrip_struct { ...@@ -82,6 +82,41 @@ struct compat_x25_subscrip_struct {
}; };
#endif #endif
int x25_parse_address_block(struct sk_buff *skb,
struct x25_address *called_addr,
struct x25_address *calling_addr)
{
unsigned char len;
int needed;
int rc;
if (skb->len < 1) {
/* packet has no address block */
rc = 0;
goto empty;
}
len = *skb->data;
needed = 1 + (len >> 4) + (len & 0x0f);
if (skb->len < needed) {
/* packet is too short to hold the addresses it claims
to hold */
rc = -1;
goto empty;
}
return x25_addr_ntoa(skb->data, called_addr, calling_addr);
empty:
*called_addr->x25_addr = 0;
*calling_addr->x25_addr = 0;
return rc;
}
int x25_addr_ntoa(unsigned char *p, struct x25_address *called_addr, int x25_addr_ntoa(unsigned char *p, struct x25_address *called_addr,
struct x25_address *calling_addr) struct x25_address *calling_addr)
{ {
...@@ -921,16 +956,26 @@ int x25_rx_call_request(struct sk_buff *skb, struct x25_neigh *nb, ...@@ -921,16 +956,26 @@ int x25_rx_call_request(struct sk_buff *skb, struct x25_neigh *nb,
/* /*
* Extract the X.25 addresses and convert them to ASCII strings, * Extract the X.25 addresses and convert them to ASCII strings,
* and remove them. * and remove them.
*
* Address block is mandatory in call request packets
*/ */
addr_len = x25_addr_ntoa(skb->data, &source_addr, &dest_addr); addr_len = x25_parse_address_block(skb, &source_addr, &dest_addr);
if (addr_len <= 0)
goto out_clear_request;
skb_pull(skb, addr_len); skb_pull(skb, addr_len);
/* /*
* Get the length of the facilities, skip past them for the moment * Get the length of the facilities, skip past them for the moment
* get the call user data because this is needed to determine * get the call user data because this is needed to determine
* the correct listener * the correct listener
*
* Facilities length is mandatory in call request packets
*/ */
if (skb->len < 1)
goto out_clear_request;
len = skb->data[0] + 1; len = skb->data[0] + 1;
if (skb->len < len)
goto out_clear_request;
skb_pull(skb,len); skb_pull(skb,len);
/* /*
......
...@@ -35,7 +35,7 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities, ...@@ -35,7 +35,7 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
struct x25_dte_facilities *dte_facs, unsigned long *vc_fac_mask) struct x25_dte_facilities *dte_facs, unsigned long *vc_fac_mask)
{ {
unsigned char *p = skb->data; unsigned char *p = skb->data;
unsigned int len = *p++; unsigned int len;
*vc_fac_mask = 0; *vc_fac_mask = 0;
...@@ -50,6 +50,14 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities, ...@@ -50,6 +50,14 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
memset(dte_facs->called_ae, '\0', sizeof(dte_facs->called_ae)); memset(dte_facs->called_ae, '\0', sizeof(dte_facs->called_ae));
memset(dte_facs->calling_ae, '\0', sizeof(dte_facs->calling_ae)); memset(dte_facs->calling_ae, '\0', sizeof(dte_facs->calling_ae));
if (skb->len < 1)
return 0;
len = *p++;
if (len >= skb->len)
return -1;
while (len > 0) { while (len > 0) {
switch (*p & X25_FAC_CLASS_MASK) { switch (*p & X25_FAC_CLASS_MASK) {
case X25_FAC_CLASS_A: case X25_FAC_CLASS_A:
...@@ -247,6 +255,8 @@ int x25_negotiate_facilities(struct sk_buff *skb, struct sock *sk, ...@@ -247,6 +255,8 @@ int x25_negotiate_facilities(struct sk_buff *skb, struct sock *sk,
memcpy(new, ours, sizeof(*new)); memcpy(new, ours, sizeof(*new));
len = x25_parse_facilities(skb, &theirs, dte, &x25->vc_facil_mask); len = x25_parse_facilities(skb, &theirs, dte, &x25->vc_facil_mask);
if (len < 0)
return len;
/* /*
* They want reverse charging, we won't accept it. * They want reverse charging, we won't accept it.
......
...@@ -89,6 +89,7 @@ static int x25_queue_rx_frame(struct sock *sk, struct sk_buff *skb, int more) ...@@ -89,6 +89,7 @@ static int x25_queue_rx_frame(struct sock *sk, struct sk_buff *skb, int more)
static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametype) static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametype)
{ {
struct x25_address source_addr, dest_addr; struct x25_address source_addr, dest_addr;
int len;
switch (frametype) { switch (frametype) {
case X25_CALL_ACCEPTED: { case X25_CALL_ACCEPTED: {
...@@ -106,11 +107,17 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp ...@@ -106,11 +107,17 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp
* Parse the data in the frame. * Parse the data in the frame.
*/ */
skb_pull(skb, X25_STD_MIN_LEN); skb_pull(skb, X25_STD_MIN_LEN);
skb_pull(skb, x25_addr_ntoa(skb->data, &source_addr, &dest_addr));
skb_pull(skb, len = x25_parse_address_block(skb, &source_addr,
x25_parse_facilities(skb, &x25->facilities, &dest_addr);
if (len > 0)
skb_pull(skb, len);
len = x25_parse_facilities(skb, &x25->facilities,
&x25->dte_facilities, &x25->dte_facilities,
&x25->vc_facil_mask)); &x25->vc_facil_mask);
if (len > 0)
skb_pull(skb, len);
/* /*
* Copy any Call User Data. * Copy any Call User Data.
*/ */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment