Skip to content

Commit

Permalink
Add fix and test for bech32 bad input
Browse files Browse the repository at this point in the history
Thanks to Christian Reitter and Dr. Jochen Hoenicke for finding the
issue.
  • Loading branch information
jgriffiths committed Oct 26, 2018
1 parent 34bac73 commit c79eb6f
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 2 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ src/stamp-h1
src/test/Makefile
src/test/Makefile.in
src/wallycore.pc
src/test_bech32*
src/test_clear*
src/test_tx*
src/test_elements_tx*
Expand Down
5 changes: 5 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Version 0.6.5

- Invalid bech32 addresses may have caused an out of bounds read. Thanks to
Christian Reitter and Dr. Jochen Hoenicke for finding and reporting this
issue. All users are advised to upgrade as soon as possible to minimise
any potential impact.

- BIP38_KEY_TESTNET was changed to reflect the testnet network version. BIP38 testnet keys
created with older versions of wally were not valid for testnet.

Expand Down
5 changes: 5 additions & 0 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ SUBDIRS = secp256k1
TESTS =
noinst_PROGRAMS =
if RUN_TESTS
TESTS += test_bech32
noinst_PROGRAMS += test_bech32
test_bech32_SOURCES = ctest/test_bech32.c
test_bech32_CFLAGS = -I$(top_srcdir)/include $(AM_CFLAGS)
test_bech32_LDADD = $(lib_LTLIBRARIES) @CTEST_EXTRA_STATIC@
if USE_PTHREAD
TESTS += test_clear
noinst_PROGRAMS += test_clear
Expand Down
4 changes: 2 additions & 2 deletions src/bech32.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ static int bech32_decode(char *hrp, uint8_t *data, size_t *data_len, const char
while (*data_len < input_len && input[(input_len - 1) - *data_len] != '1') {
++(*data_len);
}
hrp_len = input_len - (1 + *data_len);
if (hrp_len < 1 || *data_len < 6) {
if (1 + *data_len >= input_len || *data_len < 6) {
return 0;
}
hrp_len = input_len - (1 + *data_len);
*(data_len) -= 6;
for (i = 0; i < hrp_len; ++i) {
int ch = input[i];
Expand Down
37 changes: 37 additions & 0 deletions src/ctest/test_bech32.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#include "config.h"

#include <wally_core.h>
#include <wally_address.h>
#include <stdio.h>
#include <stdbool.h>

static const char *invalid = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefg";

static bool check_segwit_to_bytes()
{
unsigned char *mem = calloc(90, sizeof(unsigned char));
size_t written;
int ret;

if (!mem)
return false;

ret = wally_addr_segwit_to_bytes(invalid, "tb", 0, mem, 90, &written);

if (ret != WALLY_EINVAL)
return false;

return true;
}

int main(void)
{
bool tests_ok = true;

if (!check_segwit_to_bytes()) {
printf("check_segwit_to_bytes test failed!\n");
tests_ok = false;
}

return tests_ok ? 0 : 1;
}
5 changes: 5 additions & 0 deletions src/test/test_bech32.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ def test_segwit_address(self):
ret, result_script_hex = self.decode(addr, family)
self.assertEqual(ret, WALLY_EINVAL)

out, out_len = make_cbuffer('00' * (32 + 2))
bad = 'abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefg'
ret, written = wally_addr_segwit_to_bytes(utf8(bad), utf8('tb'), 0, out, out_len)
self.assertEqual((ret, written), (WALLY_EINVAL, 0))


if __name__ == '__main__':
unittest.main()
1 change: 1 addition & 0 deletions tools/cleanup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ rm -f src/*pyc
rm -f src/test/*pyc
rm -f src/config.h.in
rm -rf src/lcov*
rm -f src/test_bech32*
rm -f src/test_clear*
rm -f src/test_tx*
rm -f src/test-suite.log
Expand Down

0 comments on commit c79eb6f

Please sign in to comment.