Bug #119111 for Data-UUID-LibUUID: blindly decodes invalid base64

It looks like this module blindly passes any suitably long string to MIME::Base64::decode_base64() and treats the result as a UUID even if the decoding failed and produced a shorter string. The decoding process silently ignores illegal base64 characters and padding after '='. This has been observed to cause a failure of t/basic.t test 28 on platforms where Perl pointers stringify to a suitably long string, such as "Blah=HASH(0x555555f30d18)". See <https://bugs.debian.org/814929>. Proposed patch attached, adding test cases showing the behaviour and fixing it by sanity checking the length of the decoding result. Thanks for your work on free software, -- Niko Tyni ntyni@debian.org

From 2fd399a2f97156f5b643865d31787e1d74bd4577 Mon Sep 17 00:00:00 2001 From: Niko Tyni <ntyni@debian.org> Date: Sun, 27 Nov 2016 13:58:29 +0200 Subject: [PATCH 1/2] TODO tests for base64 decoding failures When the input is a suitably long string (24 to 26 characters), sv_to_uuid() decodes it as base64 but doesn't check if the result makes sense. The decoding process silently ignores illegal base64 characters and padding after '='. This can break test 28 when Perl pointers stringify to a suitably long string, such as "Blah=HASH(0x555555f30d18)". Add TODO tests showing the behaviour on all platforms. Bug-Debian: https://bugs.debian.org/814929 --- t/basic.t | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/t/basic.t b/t/basic.t index bcba897..067b8b6 100644 --- a/t/basic.t +++ b/t/basic.t @@ -2,7 +2,7 @@ use strict; -use Test::More tests => 47; +use Test::More tests => 50; use ok 'Data::UUID::LibUUID' => ":all"; @@ -70,6 +70,11 @@ is( uuid_to_binary(*STDOUT), undef, "to_binary(*STDOUT)" ); is( uuid_to_binary(sub { }), undef, "to_binary(sub { })" ); is( uuid_to_binary(42), undef, "to_binary(IV)" ); +for (19..21) { + local $::TODO = 'suitably long strings get blindly decoded (Debian #814929)'; + is( uuid_to_binary("Blah=" . "x" x $_), undef, "to_binary(string_with_${_}_padding)"); +} + is( length(new_dce_uuid_string()), 36, 'new_dce_uuid_string ignores its args' ); is( length(new_dce_uuid_string( bless({}, "Foo"), "foo" )), 36, 'new_dce_uuid_string ignores its args' ); -- 2.10.2

From 2cabe01cce645673f427be30a34654f3af40b304 Mon Sep 17 00:00:00 2001 From: Niko Tyni <ntyni@debian.org> Date: Sun, 27 Nov 2016 13:48:40 +0200 Subject: [PATCH 2/2] Check that a base64 decoded string is long enough to be a UUID This fixes test failures on platforms where Perl pointers stringify to a suitably long string, making "Blah=HASH(0x555555f30d18)" a candidate for base64 decoding. Bug-Debian: https://bugs.debian.org/814929 --- LibUUID.xs | 6 +++++- t/basic.t | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/LibUUID.xs b/LibUUID.xs index 00b4e4c..34d757d 100644 --- a/LibUUID.xs +++ b/LibUUID.xs @@ -139,7 +139,11 @@ STATIC IV sv_to_uuid (SV *sv, uuid_t uuid) { call_pv("MIME::Base64::decode_base64", G_SCALAR); SPAGAIN; - pv = SvPV_nolen(TOPs); + pv = SvPV(TOPs, len); + + /* check that the decoded result looks plausible */ + if (len != sizeof(uuid_t)) + return 0; /* fall through */ case sizeof(uuid_t): diff --git a/t/basic.t b/t/basic.t index 067b8b6..0dcbacc 100644 --- a/t/basic.t +++ b/t/basic.t @@ -71,7 +71,6 @@ is( uuid_to_binary(sub { }), undef, "to_binary(sub { })" ); is( uuid_to_binary(42), undef, "to_binary(IV)" ); for (19..21) { - local $::TODO = 'suitably long strings get blindly decoded (Debian #814929)'; is( uuid_to_binary("Blah=" . "x" x $_), undef, "to_binary(string_with_${_}_padding)"); } -- 2.10.2