slip: bound decode() reads against the compressed packet length
slhc_uncompress() parses a VJ-compressed TCP header by advancing a
pointer through the packet via decode() and pull16(). Neither helper
bounds-checks against isize, and decode() masks its return with
& 0xffff so it can never return the -1 that callers test for -- those
error paths are dead code.
A short compressed frame whose change byte requests optional fields
lets decode() read past the end of the packet. The over-read bytes
are folded into the cached cstate and reflected into subsequent
reconstructed packets.
Make decode() and pull16() take the packet end pointer and return -1
when exhausted. Add a bounds check before the TCP-checksum read.
The existing == -1 tests now do what they were always meant to.
Fixes: 1da177e4c3 ("Linux-2.6.12-rc2")
Reported-by: Simon Horman <horms@kernel.org>
Closes: https://lore.kernel.org/netdev/20260414134126.758795-2-horms@kernel.org/
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260416100147.531855-5-bestswngs@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
master
parent
e76607442d
commit
4c1367a2d7
|
|
@ -80,9 +80,9 @@
|
|||
#include <linux/unaligned.h>
|
||||
|
||||
static unsigned char *encode(unsigned char *cp, unsigned short n);
|
||||
static long decode(unsigned char **cpp);
|
||||
static long decode(unsigned char **cpp, const unsigned char *end);
|
||||
static unsigned char * put16(unsigned char *cp, unsigned short x);
|
||||
static unsigned short pull16(unsigned char **cpp);
|
||||
static long pull16(unsigned char **cpp, const unsigned char *end);
|
||||
|
||||
/* Allocate compression data structure
|
||||
* slots must be in range 0 to 255 (zero meaning no compression)
|
||||
|
|
@ -190,30 +190,34 @@ encode(unsigned char *cp, unsigned short n)
|
|||
return cp;
|
||||
}
|
||||
|
||||
/* Pull a 16-bit integer in host order from buffer in network byte order */
|
||||
static unsigned short
|
||||
pull16(unsigned char **cpp)
|
||||
/* Pull a 16-bit integer in host order from buffer in network byte order.
|
||||
* Returns -1 if the buffer is exhausted, otherwise the 16-bit value.
|
||||
*/
|
||||
static long
|
||||
pull16(unsigned char **cpp, const unsigned char *end)
|
||||
{
|
||||
short rval;
|
||||
long rval;
|
||||
|
||||
if (*cpp + 2 > end)
|
||||
return -1;
|
||||
rval = *(*cpp)++;
|
||||
rval <<= 8;
|
||||
rval |= *(*cpp)++;
|
||||
return rval;
|
||||
}
|
||||
|
||||
/* Decode a number */
|
||||
/* Decode a number. Returns -1 if the buffer is exhausted. */
|
||||
static long
|
||||
decode(unsigned char **cpp)
|
||||
decode(unsigned char **cpp, const unsigned char *end)
|
||||
{
|
||||
int x;
|
||||
|
||||
if (*cpp >= end)
|
||||
return -1;
|
||||
x = *(*cpp)++;
|
||||
if(x == 0){
|
||||
return pull16(cpp) & 0xffff; /* pull16 returns -1 on error */
|
||||
} else {
|
||||
return x & 0xff; /* -1 if PULLCHAR returned error */
|
||||
}
|
||||
if (x == 0)
|
||||
return pull16(cpp, end);
|
||||
return x & 0xff;
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
@ -499,6 +503,7 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
|
|||
struct cstate *cs;
|
||||
int len, hdrlen;
|
||||
unsigned char *cp = icp;
|
||||
const unsigned char *end = icp + isize;
|
||||
|
||||
/* We've got a compressed packet; read the change byte */
|
||||
comp->sls_i_compressed++;
|
||||
|
|
@ -536,6 +541,8 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
|
|||
thp = &cs->cs_tcp;
|
||||
ip = &cs->cs_ip;
|
||||
|
||||
if (cp + 2 > end)
|
||||
goto bad;
|
||||
thp->check = *(__sum16 *)cp;
|
||||
cp += 2;
|
||||
|
||||
|
|
@ -566,26 +573,26 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
|
|||
default:
|
||||
if(changes & NEW_U){
|
||||
thp->urg = 1;
|
||||
if((x = decode(&cp)) == -1) {
|
||||
if((x = decode(&cp, end)) == -1) {
|
||||
goto bad;
|
||||
}
|
||||
thp->urg_ptr = htons(x);
|
||||
} else
|
||||
thp->urg = 0;
|
||||
if(changes & NEW_W){
|
||||
if((x = decode(&cp)) == -1) {
|
||||
if((x = decode(&cp, end)) == -1) {
|
||||
goto bad;
|
||||
}
|
||||
thp->window = htons( ntohs(thp->window) + x);
|
||||
}
|
||||
if(changes & NEW_A){
|
||||
if((x = decode(&cp)) == -1) {
|
||||
if((x = decode(&cp, end)) == -1) {
|
||||
goto bad;
|
||||
}
|
||||
thp->ack_seq = htonl( ntohl(thp->ack_seq) + x);
|
||||
}
|
||||
if(changes & NEW_S){
|
||||
if((x = decode(&cp)) == -1) {
|
||||
if((x = decode(&cp, end)) == -1) {
|
||||
goto bad;
|
||||
}
|
||||
thp->seq = htonl( ntohl(thp->seq) + x);
|
||||
|
|
@ -593,7 +600,7 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
|
|||
break;
|
||||
}
|
||||
if(changes & NEW_I){
|
||||
if((x = decode(&cp)) == -1) {
|
||||
if((x = decode(&cp, end)) == -1) {
|
||||
goto bad;
|
||||
}
|
||||
ip->id = htons (ntohs (ip->id) + x);
|
||||
|
|
|
|||
Loading…
Reference in New Issue