diff options
author | John Hughes <john@calva.com> | 2010-04-08 00:29:25 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2010-04-08 00:29:25 -0400 |
commit | f5eb917b861828da18dc28854308068c66d1449a (patch) | |
tree | aa45d1a809abbe426b55dc89b8167e5a6609d418 | |
parent | fd218cf9557b9bf7061365a8fe7020a56d3f767c (diff) |
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: John Hughes <john@calva.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | include/net/x25.h | 4 | ||||
-rw-r--r-- | net/x25/af_x25.c | 47 | ||||
-rw-r--r-- | net/x25/x25_facilities.c | 12 | ||||
-rw-r--r-- | net/x25/x25_in.c | 15 |
4 files changed, 72 insertions, 6 deletions
diff --git a/include/net/x25.h b/include/net/x25.h index 9baa07dc7d17..33f67fb78586 100644 --- a/include/net/x25.h +++ b/include/net/x25.h | |||
@@ -182,6 +182,10 @@ extern int sysctl_x25_clear_request_timeout; | |||
182 | extern int sysctl_x25_ack_holdback_timeout; | 182 | extern int sysctl_x25_ack_holdback_timeout; |
183 | extern int sysctl_x25_forward; | 183 | extern int sysctl_x25_forward; |
184 | 184 | ||
185 | extern int x25_parse_address_block(struct sk_buff *skb, | ||
186 | struct x25_address *called_addr, | ||
187 | struct x25_address *calling_addr); | ||
188 | |||
185 | extern int x25_addr_ntoa(unsigned char *, struct x25_address *, | 189 | extern int x25_addr_ntoa(unsigned char *, struct x25_address *, |
186 | struct x25_address *); | 190 | struct x25_address *); |
187 | extern int x25_addr_aton(unsigned char *, struct x25_address *, | 191 | extern int x25_addr_aton(unsigned char *, struct x25_address *, |
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c index 9796f3ed1edb..fe26c01ef3e6 100644 --- a/net/x25/af_x25.c +++ b/net/x25/af_x25.c | |||
@@ -82,6 +82,41 @@ struct compat_x25_subscrip_struct { | |||
82 | }; | 82 | }; |
83 | #endif | 83 | #endif |
84 | 84 | ||
85 | |||
86 | int x25_parse_address_block(struct sk_buff *skb, | ||
87 | struct x25_address *called_addr, | ||
88 | struct x25_address *calling_addr) | ||
89 | { | ||
90 | unsigned char len; | ||
91 | int needed; | ||
92 | int rc; | ||
93 | |||
94 | if (skb->len < 1) { | ||
95 | /* packet has no address block */ | ||
96 | rc = 0; | ||
97 | goto empty; | ||
98 | } | ||
99 | |||
100 | len = *skb->data; | ||
101 | needed = 1 + (len >> 4) + (len & 0x0f); | ||
102 | |||
103 | if (skb->len < needed) { | ||
104 | /* packet is too short to hold the addresses it claims | ||
105 | to hold */ | ||
106 | rc = -1; | ||
107 | goto empty; | ||
108 | } | ||
109 | |||
110 | return x25_addr_ntoa(skb->data, called_addr, calling_addr); | ||
111 | |||
112 | empty: | ||
113 | *called_addr->x25_addr = 0; | ||
114 | *calling_addr->x25_addr = 0; | ||
115 | |||
116 | return rc; | ||
117 | } | ||
118 | |||
119 | |||
85 | int x25_addr_ntoa(unsigned char *p, struct x25_address *called_addr, | 120 | int x25_addr_ntoa(unsigned char *p, struct x25_address *called_addr, |
86 | struct x25_address *calling_addr) | 121 | struct x25_address *calling_addr) |
87 | { | 122 | { |
@@ -921,16 +956,26 @@ int x25_rx_call_request(struct sk_buff *skb, struct x25_neigh *nb, | |||
921 | /* | 956 | /* |
922 | * Extract the X.25 addresses and convert them to ASCII strings, | 957 | * Extract the X.25 addresses and convert them to ASCII strings, |
923 | * and remove them. | 958 | * and remove them. |
959 | * | ||
960 | * Address block is mandatory in call request packets | ||
924 | */ | 961 | */ |
925 | addr_len = x25_addr_ntoa(skb->data, &source_addr, &dest_addr); | 962 | addr_len = x25_parse_address_block(skb, &source_addr, &dest_addr); |
963 | if (addr_len <= 0) | ||
964 | goto out_clear_request; | ||
926 | skb_pull(skb, addr_len); | 965 | skb_pull(skb, addr_len); |
927 | 966 | ||
928 | /* | 967 | /* |
929 | * Get the length of the facilities, skip past them for the moment | 968 | * Get the length of the facilities, skip past them for the moment |
930 | * get the call user data because this is needed to determine | 969 | * get the call user data because this is needed to determine |
931 | * the correct listener | 970 | * the correct listener |
971 | * | ||
972 | * Facilities length is mandatory in call request packets | ||
932 | */ | 973 | */ |
974 | if (skb->len < 1) | ||
975 | goto out_clear_request; | ||
933 | len = skb->data[0] + 1; | 976 | len = skb->data[0] + 1; |
977 | if (skb->len < len) | ||
978 | goto out_clear_request; | ||
934 | skb_pull(skb,len); | 979 | skb_pull(skb,len); |
935 | 980 | ||
936 | /* | 981 | /* |
diff --git a/net/x25/x25_facilities.c b/net/x25/x25_facilities.c index a21f6646eb3a..a2765c6b1f1a 100644 --- a/net/x25/x25_facilities.c +++ b/net/x25/x25_facilities.c | |||
@@ -35,7 +35,7 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities, | |||
35 | struct x25_dte_facilities *dte_facs, unsigned long *vc_fac_mask) | 35 | struct x25_dte_facilities *dte_facs, unsigned long *vc_fac_mask) |
36 | { | 36 | { |
37 | unsigned char *p = skb->data; | 37 | unsigned char *p = skb->data; |
38 | unsigned int len = *p++; | 38 | unsigned int len; |
39 | 39 | ||
40 | *vc_fac_mask = 0; | 40 | *vc_fac_mask = 0; |
41 | 41 | ||
@@ -50,6 +50,14 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities, | |||
50 | memset(dte_facs->called_ae, '\0', sizeof(dte_facs->called_ae)); | 50 | memset(dte_facs->called_ae, '\0', sizeof(dte_facs->called_ae)); |
51 | memset(dte_facs->calling_ae, '\0', sizeof(dte_facs->calling_ae)); | 51 | memset(dte_facs->calling_ae, '\0', sizeof(dte_facs->calling_ae)); |
52 | 52 | ||
53 | if (skb->len < 1) | ||
54 | return 0; | ||
55 | |||
56 | len = *p++; | ||
57 | |||
58 | if (len >= skb->len) | ||
59 | return -1; | ||
60 | |||
53 | while (len > 0) { | 61 | while (len > 0) { |
54 | switch (*p & X25_FAC_CLASS_MASK) { | 62 | switch (*p & X25_FAC_CLASS_MASK) { |
55 | case X25_FAC_CLASS_A: | 63 | case X25_FAC_CLASS_A: |
@@ -247,6 +255,8 @@ int x25_negotiate_facilities(struct sk_buff *skb, struct sock *sk, | |||
247 | memcpy(new, ours, sizeof(*new)); | 255 | memcpy(new, ours, sizeof(*new)); |
248 | 256 | ||
249 | len = x25_parse_facilities(skb, &theirs, dte, &x25->vc_facil_mask); | 257 | len = x25_parse_facilities(skb, &theirs, dte, &x25->vc_facil_mask); |
258 | if (len < 0) | ||
259 | return len; | ||
250 | 260 | ||
251 | /* | 261 | /* |
252 | * They want reverse charging, we won't accept it. | 262 | * They want reverse charging, we won't accept it. |
diff --git a/net/x25/x25_in.c b/net/x25/x25_in.c index 96d922783547..b39072f3a297 100644 --- a/net/x25/x25_in.c +++ b/net/x25/x25_in.c | |||
@@ -89,6 +89,7 @@ static int x25_queue_rx_frame(struct sock *sk, struct sk_buff *skb, int more) | |||
89 | static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametype) | 89 | static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametype) |
90 | { | 90 | { |
91 | struct x25_address source_addr, dest_addr; | 91 | struct x25_address source_addr, dest_addr; |
92 | int len; | ||
92 | 93 | ||
93 | switch (frametype) { | 94 | switch (frametype) { |
94 | case X25_CALL_ACCEPTED: { | 95 | case X25_CALL_ACCEPTED: { |
@@ -106,11 +107,17 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp | |||
106 | * Parse the data in the frame. | 107 | * Parse the data in the frame. |
107 | */ | 108 | */ |
108 | skb_pull(skb, X25_STD_MIN_LEN); | 109 | skb_pull(skb, X25_STD_MIN_LEN); |
109 | skb_pull(skb, x25_addr_ntoa(skb->data, &source_addr, &dest_addr)); | 110 | |
110 | skb_pull(skb, | 111 | len = x25_parse_address_block(skb, &source_addr, |
111 | x25_parse_facilities(skb, &x25->facilities, | 112 | &dest_addr); |
113 | if (len > 0) | ||
114 | skb_pull(skb, len); | ||
115 | |||
116 | len = x25_parse_facilities(skb, &x25->facilities, | ||
112 | &x25->dte_facilities, | 117 | &x25->dte_facilities, |
113 | &x25->vc_facil_mask)); | 118 | &x25->vc_facil_mask); |
119 | if (len > 0) | ||
120 | skb_pull(skb, len); | ||
114 | /* | 121 | /* |
115 | * Copy any Call User Data. | 122 | * Copy any Call User Data. |
116 | */ | 123 | */ |