| Message ID | 20200403090944.17726-1-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] Fix off-by-one in tls-crypt-v2 client wrapping with custom metadata | expand |
Hi, On 03-04-2020 11:09, Arne Schwabe wrote: > Instead of writing at the end of the metadata buffer, the decoded > base64 data overwrites the opcode as BPTR points to the beginning > of the buffer and not the current position. Replace with BEND to > fix this off-by-one > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > src/openvpn/tls_crypt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c > index 37df2ce7..e9f9cc2a 100644 > --- a/src/openvpn/tls_crypt.c > +++ b/src/openvpn/tls_crypt.c > @@ -664,7 +664,7 @@ tls_crypt_v2_write_client_key_file(const char *filename, > (int)strlen(b64_metadata), TLS_CRYPT_V2_MAX_B64_METADATA_LEN); > } > ASSERT(buf_write(&metadata, &TLS_CRYPT_METADATA_TYPE_USER, 1)); > - int decoded_len = openvpn_base64_decode(b64_metadata, BPTR(&metadata), > + int decoded_len = openvpn_base64_decode(b64_metadata, BEND(&metadata), > BCAP(&metadata)); > if (decoded_len < 0) > { > Good catch. And apologies for the silly bug. Patch looks good, but it would have been nice to add a regression (unit) test. Are you willing to write one? Otherwise I will. Acked-by: Steffan Karger <steffan.karger@foxcrypto.com> -Steffan
Your patch has been applied to the master branch.
While a bugfix, the code in question is not in release/2.4, so no need
to backport.
No real testing done, just a quick test compile.
commit e23fb6b8c88a2aec160965769f6467d455c0d010 (master)
Author: Arne Schwabe
Date: Fri Apr 3 11:09:44 2020 +0200
Fix off-by-one in tls-crypt-v2 client wrapping with custom metadata
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20200403090944.17726-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19695.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c index 37df2ce7..e9f9cc2a 100644 --- a/src/openvpn/tls_crypt.c +++ b/src/openvpn/tls_crypt.c @@ -664,7 +664,7 @@ tls_crypt_v2_write_client_key_file(const char *filename, (int)strlen(b64_metadata), TLS_CRYPT_V2_MAX_B64_METADATA_LEN); } ASSERT(buf_write(&metadata, &TLS_CRYPT_METADATA_TYPE_USER, 1)); - int decoded_len = openvpn_base64_decode(b64_metadata, BPTR(&metadata), + int decoded_len = openvpn_base64_decode(b64_metadata, BEND(&metadata), BCAP(&metadata)); if (decoded_len < 0) {
Instead of writing at the end of the metadata buffer, the decoded base64 data overwrites the opcode as BPTR points to the beginning of the buffer and not the current position. Replace with BEND to fix this off-by-one Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/tls_crypt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)