From 64d5fa9ff4266ea270dd2e4e9a186eef0956d2cb Mon Sep 17 00:00:00 2001 From: orange-snn Date: Thu, 4 Jun 2020 15:03:56 +0800 Subject: [PATCH 1/4] fix heap buffer overflow in kex.c --- ...buffer-overflow-in-kex_agree_methods.patch | 118 ++++++++++++++++++ libssh2.spec | 9 +- 2 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 fix-heap-buffer-overflow-in-kex_agree_methods.patch diff --git a/fix-heap-buffer-overflow-in-kex_agree_methods.patch b/fix-heap-buffer-overflow-in-kex_agree_methods.patch new file mode 100644 index 0000000..2c2072b --- /dev/null +++ b/fix-heap-buffer-overflow-in-kex_agree_methods.patch @@ -0,0 +1,118 @@ +From 43f24eb152b8ec62473d2de6108d7c0b267b2419 Mon Sep 17 00:00:00 2001 +From: Will Cosgrove +Date: Tue, 27 Aug 2019 10:58:52 -0700 +Subject: [PATCH] kex.c: improve bounds checking in kex_agree_methods() (#399) + +file: kex.c + +notes: +use _libssh2_get_string instead of kex_string_pair which does additional checks +--- + src/kex.c | 65 ++++++++++++++++++++----------------------------------- + 1 file changed, 24 insertions(+), 41 deletions(-) + +diff --git a/src/kex.c b/src/kex.c +index df9a4fdd6..7b111feaa 100644 +--- a/src/kex.c ++++ b/src/kex.c +@@ -3937,35 +3937,10 @@ static int kex_agree_comp(LIBSSH2_SESSION *session, + } + + +- + /* TODO: When in server mode we need to turn this logic on its head + * The Client gets to make the final call on "agreed methods" + */ + +-/* +- * kex_string_pair() extracts a string from the packet and makes sure it fits +- * within the given packet. +- */ +-static int kex_string_pair(unsigned char **sp, /* parsing position */ +- unsigned char *data, /* start pointer to packet */ +- size_t data_len, /* size of total packet */ +- size_t *lenp, /* length of the string */ +- unsigned char **strp) /* pointer to string start */ +-{ +- unsigned char *s = *sp; +- *lenp = _libssh2_ntohu32(s); +- +- /* the length of the string must fit within the current pointer and the +- end of the packet */ +- if(*lenp > (data_len - (s - data) -4)) +- return 1; +- *strp = s + 4; +- s += 4 + *lenp; +- +- *sp = s; +- return 0; +-} +- + /* kex_agree_methods + * Decide which specific method to use of the methods offered by each party + */ +@@ -3976,40 +3951,48 @@ static int kex_agree_methods(LIBSSH2_SESSION * session, unsigned char *data, + *mac_cs, *mac_sc; + size_t kex_len, hostkey_len, crypt_cs_len, crypt_sc_len, comp_cs_len; + size_t comp_sc_len, mac_cs_len, mac_sc_len; +- unsigned char *s = data; ++ struct string_buf buf; + +- /* Skip packet_type, we know it already */ +- s++; ++ if(data_len < 17) ++ return -1; ++ ++ buf.data = (unsigned char *)data; ++ buf.len = data_len; ++ buf.dataptr = buf.data; ++ buf.dataptr++; /* advance past packet type */ + + /* Skip cookie, don't worry, it's preserved in the kexinit field */ +- s += 16; ++ buf.dataptr += 16; + + /* Locate each string */ +- if(kex_string_pair(&s, data, data_len, &kex_len, &kex)) ++ if(_libssh2_get_string(&buf, &kex, &kex_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &hostkey_len, &hostkey)) ++ if(_libssh2_get_string(&buf, &hostkey, &hostkey_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &crypt_cs_len, &crypt_cs)) ++ if(_libssh2_get_string(&buf, &crypt_cs, &crypt_cs_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &crypt_sc_len, &crypt_sc)) ++ if(_libssh2_get_string(&buf, &crypt_sc, &crypt_sc_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &mac_cs_len, &mac_cs)) ++ if(_libssh2_get_string(&buf, &mac_cs, &mac_cs_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &mac_sc_len, &mac_sc)) ++ if(_libssh2_get_string(&buf, &mac_sc, &mac_sc_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &comp_cs_len, &comp_cs)) ++ if(_libssh2_get_string(&buf, &comp_cs, &comp_cs_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &comp_sc_len, &comp_sc)) ++ if(_libssh2_get_string(&buf, &comp_sc, &comp_sc_len)) + return -1; + + /* If the server sent an optimistic packet, assume that it guessed wrong. + * If the guess is determined to be right (by kex_agree_kex_hostkey) + * This flag will be reset to zero so that it's not ignored */ +- session->burn_optimistic_kexinit = *(s++); +- /* Next uint32 in packet is all zeros (reserved) */ ++ if(_libssh2_check_length(&buf, 1)) { ++ session->burn_optimistic_kexinit = *(buf.dataptr++); ++ } ++ else { ++ return -1; ++ } + +- if(data_len < (unsigned) (s - data)) +- return -1; /* short packet */ ++ /* Next uint32 in packet is all zeros (reserved) */ + + if(kex_agree_kex_hostkey(session, kex, kex_len, hostkey, hostkey_len)) { + return -1; \ No newline at end of file diff --git a/libssh2.spec b/libssh2.spec index d60a9aa..0f6d0ba 100644 --- a/libssh2.spec +++ b/libssh2.spec @@ -1,6 +1,6 @@ Name: libssh2 Version: 1.9.0 -Release: 3 +Release: 4 Summary: A library implementing the SSH2 protocol License: BSD URL: https://www.libssh2.org/ @@ -9,6 +9,7 @@ Source0: https://libssh2.org/download/libssh2-%{version}.tar.gz Patch9000: 0001-libssh2-CVE-2019-17498.patch Patch9001: 0001-libssh2-misc.c-_libssh2_ntohu32-cast-bit-shifting-40.patch Patch9002: fix-use-of-uninitialized-value-476-478.patch +Patch9003: fix-heap-buffer-overflow-in-kex_agree_methods.patch BuildRequires: coreutils findutils /usr/bin/man zlib-devel BuildRequires: gcc make sed openssl-devel > 1:1.0.1 openssh-server @@ -88,6 +89,12 @@ LC_ALL=en_US.UTF-8 make -C tests check %{_mandir}/man3/libssh2_*.3* %changelog +* Sat June 4 2020 songzifeng - 1.9.0-4 +- Type:bugfix +- Id:NA +- SUG:NA +- DESC: fix heap buffer overflow in kex.c + * Sat May 30 2020 songzifeng - 1.9.0-3 - Type:bugfix - Id:NA -- Gitee From 8519fb8886e5f37c3f08a7d43e31329c14aa50c9 Mon Sep 17 00:00:00 2001 From: orange-snn Date: Thu, 4 Jun 2020 15:36:20 +0800 Subject: [PATCH 2/4] fix heap buffer overflow --- libssh2.spec | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libssh2.spec b/libssh2.spec index 0f6d0ba..519474a 100644 --- a/libssh2.spec +++ b/libssh2.spec @@ -89,17 +89,17 @@ LC_ALL=en_US.UTF-8 make -C tests check %{_mandir}/man3/libssh2_*.3* %changelog -* Sat June 4 2020 songzifeng - 1.9.0-4 +* Thur Jun 4 2020 songzifeng - 1.9.0-4 - Type:bugfix -- Id:NA +- ID:NA - SUG:NA -- DESC: fix heap buffer overflow in kex.c +- DESC:fix heap buffer overflow in kex.c * Sat May 30 2020 songzifeng - 1.9.0-3 - Type:bugfix -- Id:NA +- ID:NA - SUG:NA -- DESC: fix use of uninitialized value in transport.c +- DESC:fix use of uninitialized value in transport.c * Sat Dec 21 2019 openEuler Buildteam - 1.9.0-2 - Type:bugfix -- Gitee From ccbe676899c368a3d3e9e9bc74066f044f3d89c7 Mon Sep 17 00:00:00 2001 From: orange-snn Date: Thu, 4 Jun 2020 15:43:30 +0800 Subject: [PATCH 3/4] fix heap buffer overflow --- libssh2.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libssh2.spec b/libssh2.spec index 519474a..f1bd8c2 100644 --- a/libssh2.spec +++ b/libssh2.spec @@ -89,7 +89,7 @@ LC_ALL=en_US.UTF-8 make -C tests check %{_mandir}/man3/libssh2_*.3* %changelog -* Thur Jun 4 2020 songzifeng - 1.9.0-4 +* Thu Jun 4 2020 songzifeng - 1.9.0-4 - Type:bugfix - ID:NA - SUG:NA -- Gitee From 84d63c959c877f6882cf6dee3144afa7730cd182 Mon Sep 17 00:00:00 2001 From: orange-snn Date: Thu, 4 Jun 2020 16:06:10 +0800 Subject: [PATCH 4/4] fix spec file format --- ...buffer-overflow-in-kex_agree_methods.patch | 237 +++++++++--------- 1 file changed, 119 insertions(+), 118 deletions(-) diff --git a/fix-heap-buffer-overflow-in-kex_agree_methods.patch b/fix-heap-buffer-overflow-in-kex_agree_methods.patch index 2c2072b..47b2674 100644 --- a/fix-heap-buffer-overflow-in-kex_agree_methods.patch +++ b/fix-heap-buffer-overflow-in-kex_agree_methods.patch @@ -1,118 +1,119 @@ -From 43f24eb152b8ec62473d2de6108d7c0b267b2419 Mon Sep 17 00:00:00 2001 -From: Will Cosgrove -Date: Tue, 27 Aug 2019 10:58:52 -0700 -Subject: [PATCH] kex.c: improve bounds checking in kex_agree_methods() (#399) - -file: kex.c - -notes: -use _libssh2_get_string instead of kex_string_pair which does additional checks ---- - src/kex.c | 65 ++++++++++++++++++++----------------------------------- - 1 file changed, 24 insertions(+), 41 deletions(-) - -diff --git a/src/kex.c b/src/kex.c -index df9a4fdd6..7b111feaa 100644 ---- a/src/kex.c -+++ b/src/kex.c -@@ -3937,35 +3937,10 @@ static int kex_agree_comp(LIBSSH2_SESSION *session, - } - - -- - /* TODO: When in server mode we need to turn this logic on its head - * The Client gets to make the final call on "agreed methods" - */ - --/* -- * kex_string_pair() extracts a string from the packet and makes sure it fits -- * within the given packet. -- */ --static int kex_string_pair(unsigned char **sp, /* parsing position */ -- unsigned char *data, /* start pointer to packet */ -- size_t data_len, /* size of total packet */ -- size_t *lenp, /* length of the string */ -- unsigned char **strp) /* pointer to string start */ --{ -- unsigned char *s = *sp; -- *lenp = _libssh2_ntohu32(s); -- -- /* the length of the string must fit within the current pointer and the -- end of the packet */ -- if(*lenp > (data_len - (s - data) -4)) -- return 1; -- *strp = s + 4; -- s += 4 + *lenp; -- -- *sp = s; -- return 0; --} -- - /* kex_agree_methods - * Decide which specific method to use of the methods offered by each party - */ -@@ -3976,40 +3951,48 @@ static int kex_agree_methods(LIBSSH2_SESSION * session, unsigned char *data, - *mac_cs, *mac_sc; - size_t kex_len, hostkey_len, crypt_cs_len, crypt_sc_len, comp_cs_len; - size_t comp_sc_len, mac_cs_len, mac_sc_len; -- unsigned char *s = data; -+ struct string_buf buf; - -- /* Skip packet_type, we know it already */ -- s++; -+ if(data_len < 17) -+ return -1; -+ -+ buf.data = (unsigned char *)data; -+ buf.len = data_len; -+ buf.dataptr = buf.data; -+ buf.dataptr++; /* advance past packet type */ - - /* Skip cookie, don't worry, it's preserved in the kexinit field */ -- s += 16; -+ buf.dataptr += 16; - - /* Locate each string */ -- if(kex_string_pair(&s, data, data_len, &kex_len, &kex)) -+ if(_libssh2_get_string(&buf, &kex, &kex_len)) - return -1; -- if(kex_string_pair(&s, data, data_len, &hostkey_len, &hostkey)) -+ if(_libssh2_get_string(&buf, &hostkey, &hostkey_len)) - return -1; -- if(kex_string_pair(&s, data, data_len, &crypt_cs_len, &crypt_cs)) -+ if(_libssh2_get_string(&buf, &crypt_cs, &crypt_cs_len)) - return -1; -- if(kex_string_pair(&s, data, data_len, &crypt_sc_len, &crypt_sc)) -+ if(_libssh2_get_string(&buf, &crypt_sc, &crypt_sc_len)) - return -1; -- if(kex_string_pair(&s, data, data_len, &mac_cs_len, &mac_cs)) -+ if(_libssh2_get_string(&buf, &mac_cs, &mac_cs_len)) - return -1; -- if(kex_string_pair(&s, data, data_len, &mac_sc_len, &mac_sc)) -+ if(_libssh2_get_string(&buf, &mac_sc, &mac_sc_len)) - return -1; -- if(kex_string_pair(&s, data, data_len, &comp_cs_len, &comp_cs)) -+ if(_libssh2_get_string(&buf, &comp_cs, &comp_cs_len)) - return -1; -- if(kex_string_pair(&s, data, data_len, &comp_sc_len, &comp_sc)) -+ if(_libssh2_get_string(&buf, &comp_sc, &comp_sc_len)) - return -1; - - /* If the server sent an optimistic packet, assume that it guessed wrong. - * If the guess is determined to be right (by kex_agree_kex_hostkey) - * This flag will be reset to zero so that it's not ignored */ -- session->burn_optimistic_kexinit = *(s++); -- /* Next uint32 in packet is all zeros (reserved) */ -+ if(_libssh2_check_length(&buf, 1)) { -+ session->burn_optimistic_kexinit = *(buf.dataptr++); -+ } -+ else { -+ return -1; -+ } - -- if(data_len < (unsigned) (s - data)) -- return -1; /* short packet */ -+ /* Next uint32 in packet is all zeros (reserved) */ - - if(kex_agree_kex_hostkey(session, kex, kex_len, hostkey, hostkey_len)) { - return -1; \ No newline at end of file +From 43f24eb152b8ec62473d2de6108d7c0b267b2419 Mon Sep 17 00:00:00 2001 +From: Will Cosgrove +Date: Tue, 27 Aug 2019 10:58:52 -0700 +Subject: [PATCH] kex.c: improve bounds checking in kex_agree_methods() (#399) + +file: kex.c + +notes: +use _libssh2_get_string instead of kex_string_pair which does additional checks +--- + src/kex.c | 65 ++++++++++++++++++++----------------------------------- + 1 file changed, 24 insertions(+), 41 deletions(-) + +diff --git a/src/kex.c b/src/kex.c +index df9a4fdd6..7b111feaa 100644 +--- a/src/kex.c ++++ b/src/kex.c +@@ -3937,35 +3937,10 @@ static int kex_agree_comp(LIBSSH2_SESSION *session, + } + + +- + /* TODO: When in server mode we need to turn this logic on its head + * The Client gets to make the final call on "agreed methods" + */ + +-/* +- * kex_string_pair() extracts a string from the packet and makes sure it fits +- * within the given packet. +- */ +-static int kex_string_pair(unsigned char **sp, /* parsing position */ +- unsigned char *data, /* start pointer to packet */ +- size_t data_len, /* size of total packet */ +- size_t *lenp, /* length of the string */ +- unsigned char **strp) /* pointer to string start */ +-{ +- unsigned char *s = *sp; +- *lenp = _libssh2_ntohu32(s); +- +- /* the length of the string must fit within the current pointer and the +- end of the packet */ +- if(*lenp > (data_len - (s - data) -4)) +- return 1; +- *strp = s + 4; +- s += 4 + *lenp; +- +- *sp = s; +- return 0; +-} +- + /* kex_agree_methods + * Decide which specific method to use of the methods offered by each party + */ +@@ -3976,40 +3951,48 @@ static int kex_agree_methods(LIBSSH2_SESSION * session, unsigned char *data, + *mac_cs, *mac_sc; + size_t kex_len, hostkey_len, crypt_cs_len, crypt_sc_len, comp_cs_len; + size_t comp_sc_len, mac_cs_len, mac_sc_len; +- unsigned char *s = data; ++ struct string_buf buf; + +- /* Skip packet_type, we know it already */ +- s++; ++ if(data_len < 17) ++ return -1; ++ ++ buf.data = (unsigned char *)data; ++ buf.len = data_len; ++ buf.dataptr = buf.data; ++ buf.dataptr++; /* advance past packet type */ + + /* Skip cookie, don't worry, it's preserved in the kexinit field */ +- s += 16; ++ buf.dataptr += 16; + + /* Locate each string */ +- if(kex_string_pair(&s, data, data_len, &kex_len, &kex)) ++ if(_libssh2_get_string(&buf, &kex, &kex_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &hostkey_len, &hostkey)) ++ if(_libssh2_get_string(&buf, &hostkey, &hostkey_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &crypt_cs_len, &crypt_cs)) ++ if(_libssh2_get_string(&buf, &crypt_cs, &crypt_cs_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &crypt_sc_len, &crypt_sc)) ++ if(_libssh2_get_string(&buf, &crypt_sc, &crypt_sc_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &mac_cs_len, &mac_cs)) ++ if(_libssh2_get_string(&buf, &mac_cs, &mac_cs_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &mac_sc_len, &mac_sc)) ++ if(_libssh2_get_string(&buf, &mac_sc, &mac_sc_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &comp_cs_len, &comp_cs)) ++ if(_libssh2_get_string(&buf, &comp_cs, &comp_cs_len)) + return -1; +- if(kex_string_pair(&s, data, data_len, &comp_sc_len, &comp_sc)) ++ if(_libssh2_get_string(&buf, &comp_sc, &comp_sc_len)) + return -1; + + /* If the server sent an optimistic packet, assume that it guessed wrong. + * If the guess is determined to be right (by kex_agree_kex_hostkey) + * This flag will be reset to zero so that it's not ignored */ +- session->burn_optimistic_kexinit = *(s++); +- /* Next uint32 in packet is all zeros (reserved) */ ++ if(_libssh2_check_length(&buf, 1)) { ++ session->burn_optimistic_kexinit = *(buf.dataptr++); ++ } ++ else { ++ return -1; ++ } + +- if(data_len < (unsigned) (s - data)) +- return -1; /* short packet */ ++ /* Next uint32 in packet is all zeros (reserved) */ + + if(kex_agree_kex_hostkey(session, kex, kex_len, hostkey, hostkey_len)) { + return -1; + -- Gitee