New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perl is confused about the string which is returned from decrypt() #5

Closed
AlexDaniel opened this Issue Apr 17, 2015 · 15 comments

Comments

Projects
None yet
4 participants
@AlexDaniel

AlexDaniel commented Apr 17, 2015

This problem is reproducible on v5.10.1 (and on v5.20 it seems to be OK).

Here is an archive with a short code to reproduce it.

The guys at #perl were very helpful to identify the problem.

You can see what happens here:

5.20.2

SV = PV(0xfc0d00) at 0xfe1dd8
  REFCNT = 1
  FLAGS = (PADMY,POK,pPOK)
  PV = 0x108ced0 "Bug\0\0\0\0\0\0\0\0\0\0\0\0\0"
  CUR = 16
  LEN = 18
SV = PV(0xfc0d00) at 0xfe1dd8
  REFCNT = 1
  FLAGS = (PADMY,POK,pPOK)
  PV = 0x1075b60 "Bug"\0
  CUR = 3
  LEN = 10

5.10.1 (with \z)

SV = PV(0x1226018) at 0x1257ef0
  REFCNT = 1
  FLAGS = (PADMY,POK,pPOK)
  PV = 0x1228f80 "Bug\0\0\0\0\0\0\0\0\0\0\0\0\0"
  CUR = 16
  LEN = 24
SV = PV(0x1226018) at 0x1257ef0
  REFCNT = 1
  FLAGS = (PADMY,POK,pPOK)
  PV = 0x1228f80 "Bug"\0
  CUR = 3
  LEN = 24

5.10.1 (with $)

SV = PV(0x1f1e018) at 0x1f4ef38
  REFCNT = 1
  FLAGS = (PADMY,POK,pPOK)
  PV = 0x1f20f80 "Bug\0\0\0\0\0\0\0\0\0\0\0\0\0"
  CUR = 16
  LEN = 24
SV = PV(0x1f1e018) at 0x1f4ef38
  REFCNT = 1
  FLAGS = (PADMY,POK,pPOK)
  PV = 0x1f20f80 "Bug\0\0\0\0\0\0\0\0\0\0\0\0\0"
  CUR = 16
  LEN = 24

Grinnz_ suggested to change $ in regex to \z, and it actually works (although is weird). Later it turned out that if you just copy that string somewhere else (e.g. $copy = $data) then the copy will turn normal.

Also:

02:20:30 tm604: also try http://fpaste.scsys.co.uk/472847
02:21:10 tm604: it's a hack and I'm sure there are cleaner ways to do that but if the issue is trailing \0 handling then I'd expect that to "fix" the issue on 5.10.1

If the paste expires, here is the copy:

diff --git a/Rijndael.xs b/Rijndael.xs
index 14be659..8b469e2 100644
--- a/Rijndael.xs
+++ b/Rijndael.xs
@@ -183,11 +183,12 @@ encrypt(self, data)
            if (size % RIJNDAEL_BLOCKSIZE)
              croak ("encrypt: datasize not multiple of blocksize (%d bytes)", RIJNDAEL_BLOCKSIZE);

-           RETVAL = NEWSV (0, size);
+           RETVAL = NEWSV (0, size + 1);
            SvPOK_only (RETVAL);
            SvCUR_set (RETVAL, size);
            (ix ? block_decrypt : block_encrypt)
              (&self->ctx, rawbytes, size, (UINT8 *) SvPV_nolen(RETVAL), self->iv);
+             *(size + (UINT8 *) SvPV_nolen(RETVAL)) = "\0";
           } else
             RETVAL = newSVpv ("", 0);
         }
@briandfoy

This comment has been minimized.

Show comment
Hide comment
@briandfoy

briandfoy Apr 17, 2015

Contributor
Contributor

briandfoy commented Apr 17, 2015

@AlexDaniel

This comment has been minimized.

Show comment
Hide comment
@AlexDaniel

AlexDaniel Apr 17, 2015

Well, the reason why it is archived is because there is an input file (and I wonder what is the problem with the archive, hm). Well, let's see:

#!/usr/bin/perl
use strict;
use warnings;
use v5.10;

use Crypt::Rijndael;

my $cipher = Crypt::Rijndael->new('a' x 32, Crypt::Rijndael::MODE_CBC());

if (open(IN, '<', 'crypted')) {
  local $/ = undef; # Read complete files
  my $data=<IN>;
  close IN;
  $data = $cipher->decrypt($data);
  use Devel::Peek 'Dump';
  Dump $data;
  $data =~ s/\0+$//;
  Dump $data;
}

And the contents of that file are the following (perl string):

"\372\316\330\350\214\2201\264\255\207\3\205\t\35\272."

or like this (hexdump output):

cefa e8d8 908c b431 87ad 8503 1d09 2eba

Which, after decoding, will turn into "Bug\0\0\0\0\0\0\0\0\0\0\0\0\0".

AlexDaniel commented Apr 17, 2015

Well, the reason why it is archived is because there is an input file (and I wonder what is the problem with the archive, hm). Well, let's see:

#!/usr/bin/perl
use strict;
use warnings;
use v5.10;

use Crypt::Rijndael;

my $cipher = Crypt::Rijndael->new('a' x 32, Crypt::Rijndael::MODE_CBC());

if (open(IN, '<', 'crypted')) {
  local $/ = undef; # Read complete files
  my $data=<IN>;
  close IN;
  $data = $cipher->decrypt($data);
  use Devel::Peek 'Dump';
  Dump $data;
  $data =~ s/\0+$//;
  Dump $data;
}

And the contents of that file are the following (perl string):

"\372\316\330\350\214\2201\264\255\207\3\205\t\35\272."

or like this (hexdump output):

cefa e8d8 908c b431 87ad 8503 1d09 2eba

Which, after decoding, will turn into "Bug\0\0\0\0\0\0\0\0\0\0\0\0\0".

@briandfoy

This comment has been minimized.

Show comment
Hide comment
@briandfoy

briandfoy Apr 17, 2015

Contributor
Contributor

briandfoy commented Apr 17, 2015

@AlexDaniel

This comment has been minimized.

Show comment
Hide comment
@AlexDaniel

AlexDaniel Apr 17, 2015

Uh, well, I can't see how that helps, but I just came up with something that produces the same output, so there you go:

#!/usr/bin/perl
use strict;
use warnings;
use v5.10;

use Crypt::Rijndael;

my $cipher = Crypt::Rijndael->new('a' x 32, Crypt::Rijndael::MODE_CBC());

sub PadTo16Bytes {
  return $_[0] if length($_[0]) % 16 == 0;
  return $_[0] . "\0" x (16 - length($_[0]) % 16);
}

my $text = "Bug";
print $cipher->encrypt(PadTo16Bytes $text);

AlexDaniel commented Apr 17, 2015

Uh, well, I can't see how that helps, but I just came up with something that produces the same output, so there you go:

#!/usr/bin/perl
use strict;
use warnings;
use v5.10;

use Crypt::Rijndael;

my $cipher = Crypt::Rijndael->new('a' x 32, Crypt::Rijndael::MODE_CBC());

sub PadTo16Bytes {
  return $_[0] if length($_[0]) % 16 == 0;
  return $_[0] . "\0" x (16 - length($_[0]) % 16);
}

my $text = "Bug";
print $cipher->encrypt(PadTo16Bytes $text);
@AlexDaniel

This comment has been minimized.

Show comment
Hide comment
@AlexDaniel

AlexDaniel Apr 18, 2015

Here is a 2 hour IRC log, maybe it will help you. There is a lot of irrelevant information, sorry, I tried filtering it.

00:45:57<Sysaxed> So I want to remove some trailing null bytes, I do $data =~
          s/\0+$//; (or \x0 or \x00, doesn't matter) - it works! Then
          I test the same thing on my server as cgi script - doesn't
          work. So, maybe my older perl version on the server can't
          handle it? I try it on the command line - it works! What? To
          make it even weirder, $data =~ tr/\0/X/ works! But
          substitution doesn't. How is that possible? Any ideas?
00:47:17<Grinnz_> newlines or windows carriage returns, probably
00:47:36<Grinnz_> $ will match a newline at the end of the string but not a \r
00:48:52<Sysaxed> Grinnz_: the data is exactly the same, so that's not the
          case, I think. But how do I check your idea just to be sure?
00:49:06<PerlJam> Sysaxed: how are you testing "works" vs. "doesn't work"?
          What are the inputs and outputs?  What perl versions are we
          talking about?
00:49:08<Grinnz_> well what is the "older perl version"?
00:49:17<Grinnz_> and all that other stuff, yeah
00:49:28<Sysaxed> old, Perl v5.10.1
00:49:41<Grinnz_> thats not ancient enough to do anything real funky i dont
          think
00:50:23<Grinnz_> how is the input data received, are you sure it's getting
          into $data the same way?
00:51:17<Grinnz_> you could test that by putting a \r? before the $
00:51:23<Grinnz_> (the carriage return thing)
00:53:36<Sysaxed> So the contents of $data: 84 101 115 116 52 0 0 0 0 0 0 0 0
          0 0 0
00:53:50<Grinnz_> are those bytes?
00:53:51<Sysaxed> I mean, it is a string that is 16 chars long
00:53:54<Sysaxed> and these are the bytes
00:54:54<Sysaxed> I get these bytes by doing for (my $i = 0; $i < length
          $data; $i++) { say ord(substr $data, $i, 1) }
00:54:55<Grinnz_> can you verify that with Data::Dumper directly before yo
          perform the substitution in that cgi script?
00:55:02<Soltis> Sysaxed: use Devel::Peek; Dump($data); #Compare output on CLI
         and server
        <Sysaxed> not sure if it the best way but gets the job done
00:55:12<Sysaxed> ok, Data::Dumper, good idea
00:55:41<Soltis> Sysaxed: I'd prefer Dump over Dumper in this case, though
         Dumper with Data::Dumper::Useqq=1 should work too
01:00:25<Sysaxed> $VAR1 = "Test4\0\0\0\0\0\0\0\0\0\0\0"; 
01:00:35<Sysaxed> says Data::Dumper with Useqq=1
01:00:41<Sysaxed> that's before I do any substitution
01:01:38<Sysaxed> huh what, I have some progress here...
01:02:51<Sysaxed> ah no, no progress
01:04:08<Sysaxed> exacly the same result after substitution. I've felt like it
          worked because I did s/\0+// without $ . So it seems like it
          is unable to detect the end of the line, but how is that
          possible? Maybe something is affecting it? Like was there
          any magic variable to control line endings?
01:10:30<Sysaxed> hm, so what is that magical thing that can break $ in
          regexes? I'm stuck on that simple line for more than an hour
          now...
01:10:46<LeoNerd> ?
01:10:51<LeoNerd> "magical thing that can break $"?
01:11:01<Grinnz_> Sysaxed: nothing. if your data really looks as you described
          that regex would match
01:11:22<Grinnz_> Sysaxed: however since there's no newlines involves you
          might as well use \z instead of $
01:11:59<Grinnz_> beyond that... you might have to start pasting some more
          context
01:12:34<Sysaxed> LeoNerd: I have a variable with null bytes: $VAR1 =
          "Test4\0\0\0\0\0\0\0\0\0\0\0" and I want to remove trailing
          null bytes. If I do $data =~ s/\0+//; it works, but $data =~
          s/\0+$//; does not work...
01:13:25<Sysaxed> Grinnz_: \z works
01:13:46<Grinnz_> ...alright then
01:14:00<LeoNerd> eval: $_ = "Test4\0\0\0\0\0\0\0\0\0\0\0"; $_ =~ s/\0+$//;
          $_
01:14:02<perlbot> LeoNerd: Test4
01:14:13<Grinnz_> might want to make it dumper that
01:14:14<LeoNerd> eval: $_ = "Test4\0\0\0\0\0\0\0\0\0\0\0"; $_ =~ s/\0+$//;
          sprintf "%v.02x", $_
01:14:16<perlbot> LeoNerd: 54.65.73.74.34
01:14:19<LeoNerd> ^-- WFM
01:15:14<huf> why face men
01:15:19<Grinnz_> why?????
01:15:21<ttkai> "works for me"
01:16:26<huf> ttkai: spoonerism?
01:16:28* PerlJam is still in a state of disbelief wrt Sysaxed's problem
01:16:56<Grinnz_> yeah, even CGI.pm on perl 5.10.1 shouldn't break $
01:17:18<Grinnz_> i'd assume the input data was what's breaking, but if \z
          works, i just don't know anymore
01:17:26<Sysaxed> PerlJam: well, I wish I could throw more code but there's
          too much of it, I think...
01:17:37<Sysaxed> but really, \z works and $ doesn't. What the hell is that
01:18:15<PerlJam> Sysaxed: create a simple example that illustrates "works" vs
          "doesn't work"
01:18:50<Sysaxed> PerlJam: well, I'll try that right now.
01:28:05<Sysaxed> Okay, now I feel stupid for not throwing a little bit more
          of code (turns out that not much was required). It is still
          weird though. The data I get comes from Crypt::Rijndael
          (which kinda explains why it is padded with null bytes on
          the end) and $ is broken if I use it on this data. If I
          create a string myself with just
          $data="Test4\0\0\0\0\0\0\0\0\0\0\0" then it is ok. Is there
          any logical explanation?
01:30:14<alpha-> Sysaxed post minimal self-sufficient example
01:30:28<Sysaxed> alpha-: doing it
01:30:46<Sysaxed> alpha-: although it works on debian, so I have a feeling
          that it was fixed...
01:30:54<Sysaxed> somehow
01:31:12<anno> unicode effect?
01:31:26<LeoNerd> Huh...
01:32:04<alpha-> looks like Crypt::Rijndael is XS
01:32:12<LeoNerd> I'm sure I tried to just convert it to linear before, and it
          didn't work. Now it does
01:32:15<LeoNerd> How very strange
01:32:23<alpha-> I guess the XS bits might not be playing nice
01:32:28<alpha-> wouldn't be too surprised
01:32:51<LeoNerd> Next task: work out where all my mail went :/
01:32:59<Grinnz_> circular file!
01:33:07<anno> use Devel::Peek to compare the strings
01:34:04* Grinnz_ idly wonders what version of git github runs
01:37:04<alpha-> I find it funny that github keeps own code closed
01:37:33<Grinnz_> well, it's a service, not software
01:37:57<Grinnz_> not just software, i should say
01:38:45<Sysaxed> okay, minimal example is ready
01:47:46<Sysaxed> So, that's it: https://files.progarm.org/perlaes
01:48:01<Sysaxed> on my machine it works (trailing null bytes are removed), on
          my server it doesn't
01:48:23<Sysaxed> holy crap hold on it just worked
01:48:32<Sysaxed> gah
01:48:57<Grinnz_> lol
01:50:46<Sysaxed> so it happens only if I read it from file
01:51:08<Sysaxed> https://files.progarm.org/rijndaelbug.tar.gz here is an
          archive with both encrypted data in a file and a really
          short script
01:51:43<Sysaxed> which works on my PC but doesn't work on my server
01:52:00<Sysaxed> I feel like I'm doing something dumb, huh
01:52:26<anno> use Devel::Peek to compare the strings
01:52:50<Sysaxed> oh right
01:53:08<tm604> seems okay here?
01:53:25<Grinnz_> verified on my 5.10.1, \z works and $ does not
01:53:27<tm604> output is "Bug"\0, you get something else on 5.10.1 on the
        server?
01:53:50<Sysaxed> Grinnz_: HOORAY!
01:54:08<Sysaxed> tm604: yes, the string is unmodified
01:54:35<Grinnz_> http://fpaste.org/212512/31125614/
01:55:07<Grinnz_> something weird going on with those SV lengths
01:55:24<tm604> how about -Mre=debug, anything useful there?
01:56:10<Grinnz_> a whole lot of garbage, but useful, i have no idea :P
01:56:54<Sysaxed> Grinnz_: thanks a lot, you're so helpful!
01:57:42<Grinnz_> http://fpaste.org/212514/93114561/
01:57:59<Grinnz_> that seems the relevant part
02:00:16<Grinnz_> compared to with \z: http://fpaste.org/212516/11528142/ (the
          "final program" differs in that line 4 is EOL for $ and EOS
          for \z)
02:04:06<Sysaxed> any quick way to test that code under different perl
          versions?
02:04:18<thrig> perlbrew, exec
02:04:22<Grinnz_> quick? not unless you already have perlbrews set up
02:04:32<thrig> cpan it up and wait for smoketesting
02:04:36<Grinnz_> heh
02:05:35<Sysaxed> I'm just curious if that was fixed somewhere
02:05:40<Sysaxed> and it seems like it was
02:06:00<Grinnz_> in newer perls, yes
02:06:10<Grinnz_> unsurprising, 5.10.1 is mildly ancient
02:06:39<Sysaxed> so, is there anything else I can do?
02:06:45<Grinnz_> this is why i built a new perl for my centos6 box once i
          figured out i had enough disk space for it
02:06:59<Grinnz_> use \z? that seems to work
02:07:17<Sysaxed> sure, but should I report it somewhere or something?
02:07:21<tm604> or copy the data to another scalar
02:07:36<tm604> nah, things not working on 5.10.1 is pretty much "yes, that's
        expected"
02:07:37<Grinnz_> perl 5.10.1 bugs aren't going to be acted on unless they're
          security related really
02:07:41<Grinnz_> even then...
02:07:47<Grinnz_> only by redhat probably :P
02:07:51<tm604> but if you do my $copy = "$data";, I *think* that'd make $
        work
02:08:00<Grinnz_> ah perhaps
02:08:02<tm604> if that's the case, I'm guessing the final \0 is missing
02:08:20<Sysaxed> Grinnz_: sure, but I'm not so confident about it being fixed
02:08:28<Sysaxed> it works on newer perl, but who knows why
02:08:32<tm604> (if so, Dump $_ for $data, $copy; should show an extra \0 for
        the copied version)
02:08:59<Grinnz_>   PV = 0x10bd6c0 "Bug\0\0\0\0\0\0\0\0\0\0\0\0\0"\0
02:09:02<Grinnz_> ^ copied version
02:09:16<tm604> yeah - that \0 outside the "" looks right to me, is it missing
        on the original?
02:09:20<Grinnz_> yes
02:09:23<Grinnz_> in 5.20.2 as well
02:09:51<Grinnz_> but in 5.20.2, $ matches still...
02:09:54<tm604> just wondering if ::Rijndael is constructing the scalar
        incorrectly (presumably it's coming from XS somewhere)
02:10:00<Grinnz_> yeah that entire module is XS
02:10:02<Grinnz_> so probably
02:10:58<Grinnz_> Sysaxed: i would suggest copying the output from the module
          to a new scalar, that seems to make it normal
02:12:20<tm604> "Be warned, though, that Perl will determine the string's
        length by using strlen , which depends on the string
        terminating with a NUL character, and not otherwise containing
        NULs"
02:12:32<Grinnz_> heh
02:12:33<tm604> so it shouldn't be using setpv that way, I think
02:13:04<tm604> (or whatever the code is using, haven't actually looked yet -
        that quote is from perlguts)
02:13:17<Grinnz_> Sysaxed: so it seems if you want to report a bug, agains the
          module would be where
02:16:54<Sysaxed> okay great
02:18:16<Sysaxed> "The quickest way to report a bug in Crypt-Rijndael is by
          sending email to bug-Crypt-Rijndael [at] rt.cpan.org."
02:18:22<Sysaxed> or should I report it right on github
02:18:27* Sysaxed confused
02:20:03<Sysaxed> github then
02:20:17<tm604> is crypt-rijndael on github? if so, that's probably reasonable
02:20:26<Sysaxed> yup
        <tm604> whichever place has the most issues listed currently,
        basically =)
02:20:30<tm604> also try http://fpaste.scsys.co.uk/472847
02:21:10<tm604> it's a hack and I'm sure there are cleaner ways to do that but
        if the issue is trailing \0 handling then I'd expect that to
        "fix" the issue on 5.10.1
02:31:12<Sysaxed> tm604: I've not tried it but I quoted it, if you don't mind
02:31:13<Sysaxed> https://github.com/briandfoy/crypt-rijndael/issues/5
02:32:29<tm604> sure - just note that paste site tends to expire things after
        a very short time
02:33:03<Sysaxed> ahhh okay I'll copy it
02:33:34<Sysaxed> done
02:33:48<Sysaxed> whoops, one more
02:34:43<Sysaxed> ok
02:36:30<Sysaxed> OK thanks, enough for that thing
02:36:43<Sysaxed> you guys were so helpful, thank you very much!
02:52:18<Sysaxed> tm604: I feel like that guy does not understand the problem
02:52:40<Sysaxed> but his response is so quick!
02:54:15<tm604> heh, "didn't work"! I'd expect better than that from bdfoy
02:54:50<tm604> but yes, it seems that he's treating it as an AES issue rather
        than a perl/xs issue
02:55:27<Sysaxed> maybe I should just throw the whole log from here so that he
          can feel the pain?
02:55:41<Sysaxed> :)

AlexDaniel commented Apr 18, 2015

Here is a 2 hour IRC log, maybe it will help you. There is a lot of irrelevant information, sorry, I tried filtering it.

00:45:57<Sysaxed> So I want to remove some trailing null bytes, I do $data =~
          s/\0+$//; (or \x0 or \x00, doesn't matter) - it works! Then
          I test the same thing on my server as cgi script - doesn't
          work. So, maybe my older perl version on the server can't
          handle it? I try it on the command line - it works! What? To
          make it even weirder, $data =~ tr/\0/X/ works! But
          substitution doesn't. How is that possible? Any ideas?
00:47:17<Grinnz_> newlines or windows carriage returns, probably
00:47:36<Grinnz_> $ will match a newline at the end of the string but not a \r
00:48:52<Sysaxed> Grinnz_: the data is exactly the same, so that's not the
          case, I think. But how do I check your idea just to be sure?
00:49:06<PerlJam> Sysaxed: how are you testing "works" vs. "doesn't work"?
          What are the inputs and outputs?  What perl versions are we
          talking about?
00:49:08<Grinnz_> well what is the "older perl version"?
00:49:17<Grinnz_> and all that other stuff, yeah
00:49:28<Sysaxed> old, Perl v5.10.1
00:49:41<Grinnz_> thats not ancient enough to do anything real funky i dont
          think
00:50:23<Grinnz_> how is the input data received, are you sure it's getting
          into $data the same way?
00:51:17<Grinnz_> you could test that by putting a \r? before the $
00:51:23<Grinnz_> (the carriage return thing)
00:53:36<Sysaxed> So the contents of $data: 84 101 115 116 52 0 0 0 0 0 0 0 0
          0 0 0
00:53:50<Grinnz_> are those bytes?
00:53:51<Sysaxed> I mean, it is a string that is 16 chars long
00:53:54<Sysaxed> and these are the bytes
00:54:54<Sysaxed> I get these bytes by doing for (my $i = 0; $i < length
          $data; $i++) { say ord(substr $data, $i, 1) }
00:54:55<Grinnz_> can you verify that with Data::Dumper directly before yo
          perform the substitution in that cgi script?
00:55:02<Soltis> Sysaxed: use Devel::Peek; Dump($data); #Compare output on CLI
         and server
        <Sysaxed> not sure if it the best way but gets the job done
00:55:12<Sysaxed> ok, Data::Dumper, good idea
00:55:41<Soltis> Sysaxed: I'd prefer Dump over Dumper in this case, though
         Dumper with Data::Dumper::Useqq=1 should work too
01:00:25<Sysaxed> $VAR1 = "Test4\0\0\0\0\0\0\0\0\0\0\0"; 
01:00:35<Sysaxed> says Data::Dumper with Useqq=1
01:00:41<Sysaxed> that's before I do any substitution
01:01:38<Sysaxed> huh what, I have some progress here...
01:02:51<Sysaxed> ah no, no progress
01:04:08<Sysaxed> exacly the same result after substitution. I've felt like it
          worked because I did s/\0+// without $ . So it seems like it
          is unable to detect the end of the line, but how is that
          possible? Maybe something is affecting it? Like was there
          any magic variable to control line endings?
01:10:30<Sysaxed> hm, so what is that magical thing that can break $ in
          regexes? I'm stuck on that simple line for more than an hour
          now...
01:10:46<LeoNerd> ?
01:10:51<LeoNerd> "magical thing that can break $"?
01:11:01<Grinnz_> Sysaxed: nothing. if your data really looks as you described
          that regex would match
01:11:22<Grinnz_> Sysaxed: however since there's no newlines involves you
          might as well use \z instead of $
01:11:59<Grinnz_> beyond that... you might have to start pasting some more
          context
01:12:34<Sysaxed> LeoNerd: I have a variable with null bytes: $VAR1 =
          "Test4\0\0\0\0\0\0\0\0\0\0\0" and I want to remove trailing
          null bytes. If I do $data =~ s/\0+//; it works, but $data =~
          s/\0+$//; does not work...
01:13:25<Sysaxed> Grinnz_: \z works
01:13:46<Grinnz_> ...alright then
01:14:00<LeoNerd> eval: $_ = "Test4\0\0\0\0\0\0\0\0\0\0\0"; $_ =~ s/\0+$//;
          $_
01:14:02<perlbot> LeoNerd: Test4
01:14:13<Grinnz_> might want to make it dumper that
01:14:14<LeoNerd> eval: $_ = "Test4\0\0\0\0\0\0\0\0\0\0\0"; $_ =~ s/\0+$//;
          sprintf "%v.02x", $_
01:14:16<perlbot> LeoNerd: 54.65.73.74.34
01:14:19<LeoNerd> ^-- WFM
01:15:14<huf> why face men
01:15:19<Grinnz_> why?????
01:15:21<ttkai> "works for me"
01:16:26<huf> ttkai: spoonerism?
01:16:28* PerlJam is still in a state of disbelief wrt Sysaxed's problem
01:16:56<Grinnz_> yeah, even CGI.pm on perl 5.10.1 shouldn't break $
01:17:18<Grinnz_> i'd assume the input data was what's breaking, but if \z
          works, i just don't know anymore
01:17:26<Sysaxed> PerlJam: well, I wish I could throw more code but there's
          too much of it, I think...
01:17:37<Sysaxed> but really, \z works and $ doesn't. What the hell is that
01:18:15<PerlJam> Sysaxed: create a simple example that illustrates "works" vs
          "doesn't work"
01:18:50<Sysaxed> PerlJam: well, I'll try that right now.
01:28:05<Sysaxed> Okay, now I feel stupid for not throwing a little bit more
          of code (turns out that not much was required). It is still
          weird though. The data I get comes from Crypt::Rijndael
          (which kinda explains why it is padded with null bytes on
          the end) and $ is broken if I use it on this data. If I
          create a string myself with just
          $data="Test4\0\0\0\0\0\0\0\0\0\0\0" then it is ok. Is there
          any logical explanation?
01:30:14<alpha-> Sysaxed post minimal self-sufficient example
01:30:28<Sysaxed> alpha-: doing it
01:30:46<Sysaxed> alpha-: although it works on debian, so I have a feeling
          that it was fixed...
01:30:54<Sysaxed> somehow
01:31:12<anno> unicode effect?
01:31:26<LeoNerd> Huh...
01:32:04<alpha-> looks like Crypt::Rijndael is XS
01:32:12<LeoNerd> I'm sure I tried to just convert it to linear before, and it
          didn't work. Now it does
01:32:15<LeoNerd> How very strange
01:32:23<alpha-> I guess the XS bits might not be playing nice
01:32:28<alpha-> wouldn't be too surprised
01:32:51<LeoNerd> Next task: work out where all my mail went :/
01:32:59<Grinnz_> circular file!
01:33:07<anno> use Devel::Peek to compare the strings
01:34:04* Grinnz_ idly wonders what version of git github runs
01:37:04<alpha-> I find it funny that github keeps own code closed
01:37:33<Grinnz_> well, it's a service, not software
01:37:57<Grinnz_> not just software, i should say
01:38:45<Sysaxed> okay, minimal example is ready
01:47:46<Sysaxed> So, that's it: https://files.progarm.org/perlaes
01:48:01<Sysaxed> on my machine it works (trailing null bytes are removed), on
          my server it doesn't
01:48:23<Sysaxed> holy crap hold on it just worked
01:48:32<Sysaxed> gah
01:48:57<Grinnz_> lol
01:50:46<Sysaxed> so it happens only if I read it from file
01:51:08<Sysaxed> https://files.progarm.org/rijndaelbug.tar.gz here is an
          archive with both encrypted data in a file and a really
          short script
01:51:43<Sysaxed> which works on my PC but doesn't work on my server
01:52:00<Sysaxed> I feel like I'm doing something dumb, huh
01:52:26<anno> use Devel::Peek to compare the strings
01:52:50<Sysaxed> oh right
01:53:08<tm604> seems okay here?
01:53:25<Grinnz_> verified on my 5.10.1, \z works and $ does not
01:53:27<tm604> output is "Bug"\0, you get something else on 5.10.1 on the
        server?
01:53:50<Sysaxed> Grinnz_: HOORAY!
01:54:08<Sysaxed> tm604: yes, the string is unmodified
01:54:35<Grinnz_> http://fpaste.org/212512/31125614/
01:55:07<Grinnz_> something weird going on with those SV lengths
01:55:24<tm604> how about -Mre=debug, anything useful there?
01:56:10<Grinnz_> a whole lot of garbage, but useful, i have no idea :P
01:56:54<Sysaxed> Grinnz_: thanks a lot, you're so helpful!
01:57:42<Grinnz_> http://fpaste.org/212514/93114561/
01:57:59<Grinnz_> that seems the relevant part
02:00:16<Grinnz_> compared to with \z: http://fpaste.org/212516/11528142/ (the
          "final program" differs in that line 4 is EOL for $ and EOS
          for \z)
02:04:06<Sysaxed> any quick way to test that code under different perl
          versions?
02:04:18<thrig> perlbrew, exec
02:04:22<Grinnz_> quick? not unless you already have perlbrews set up
02:04:32<thrig> cpan it up and wait for smoketesting
02:04:36<Grinnz_> heh
02:05:35<Sysaxed> I'm just curious if that was fixed somewhere
02:05:40<Sysaxed> and it seems like it was
02:06:00<Grinnz_> in newer perls, yes
02:06:10<Grinnz_> unsurprising, 5.10.1 is mildly ancient
02:06:39<Sysaxed> so, is there anything else I can do?
02:06:45<Grinnz_> this is why i built a new perl for my centos6 box once i
          figured out i had enough disk space for it
02:06:59<Grinnz_> use \z? that seems to work
02:07:17<Sysaxed> sure, but should I report it somewhere or something?
02:07:21<tm604> or copy the data to another scalar
02:07:36<tm604> nah, things not working on 5.10.1 is pretty much "yes, that's
        expected"
02:07:37<Grinnz_> perl 5.10.1 bugs aren't going to be acted on unless they're
          security related really
02:07:41<Grinnz_> even then...
02:07:47<Grinnz_> only by redhat probably :P
02:07:51<tm604> but if you do my $copy = "$data";, I *think* that'd make $
        work
02:08:00<Grinnz_> ah perhaps
02:08:02<tm604> if that's the case, I'm guessing the final \0 is missing
02:08:20<Sysaxed> Grinnz_: sure, but I'm not so confident about it being fixed
02:08:28<Sysaxed> it works on newer perl, but who knows why
02:08:32<tm604> (if so, Dump $_ for $data, $copy; should show an extra \0 for
        the copied version)
02:08:59<Grinnz_>   PV = 0x10bd6c0 "Bug\0\0\0\0\0\0\0\0\0\0\0\0\0"\0
02:09:02<Grinnz_> ^ copied version
02:09:16<tm604> yeah - that \0 outside the "" looks right to me, is it missing
        on the original?
02:09:20<Grinnz_> yes
02:09:23<Grinnz_> in 5.20.2 as well
02:09:51<Grinnz_> but in 5.20.2, $ matches still...
02:09:54<tm604> just wondering if ::Rijndael is constructing the scalar
        incorrectly (presumably it's coming from XS somewhere)
02:10:00<Grinnz_> yeah that entire module is XS
02:10:02<Grinnz_> so probably
02:10:58<Grinnz_> Sysaxed: i would suggest copying the output from the module
          to a new scalar, that seems to make it normal
02:12:20<tm604> "Be warned, though, that Perl will determine the string's
        length by using strlen , which depends on the string
        terminating with a NUL character, and not otherwise containing
        NULs"
02:12:32<Grinnz_> heh
02:12:33<tm604> so it shouldn't be using setpv that way, I think
02:13:04<tm604> (or whatever the code is using, haven't actually looked yet -
        that quote is from perlguts)
02:13:17<Grinnz_> Sysaxed: so it seems if you want to report a bug, agains the
          module would be where
02:16:54<Sysaxed> okay great
02:18:16<Sysaxed> "The quickest way to report a bug in Crypt-Rijndael is by
          sending email to bug-Crypt-Rijndael [at] rt.cpan.org."
02:18:22<Sysaxed> or should I report it right on github
02:18:27* Sysaxed confused
02:20:03<Sysaxed> github then
02:20:17<tm604> is crypt-rijndael on github? if so, that's probably reasonable
02:20:26<Sysaxed> yup
        <tm604> whichever place has the most issues listed currently,
        basically =)
02:20:30<tm604> also try http://fpaste.scsys.co.uk/472847
02:21:10<tm604> it's a hack and I'm sure there are cleaner ways to do that but
        if the issue is trailing \0 handling then I'd expect that to
        "fix" the issue on 5.10.1
02:31:12<Sysaxed> tm604: I've not tried it but I quoted it, if you don't mind
02:31:13<Sysaxed> https://github.com/briandfoy/crypt-rijndael/issues/5
02:32:29<tm604> sure - just note that paste site tends to expire things after
        a very short time
02:33:03<Sysaxed> ahhh okay I'll copy it
02:33:34<Sysaxed> done
02:33:48<Sysaxed> whoops, one more
02:34:43<Sysaxed> ok
02:36:30<Sysaxed> OK thanks, enough for that thing
02:36:43<Sysaxed> you guys were so helpful, thank you very much!
02:52:18<Sysaxed> tm604: I feel like that guy does not understand the problem
02:52:40<Sysaxed> but his response is so quick!
02:54:15<tm604> heh, "didn't work"! I'd expect better than that from bdfoy
02:54:50<tm604> but yes, it seems that he's treating it as an AES issue rather
        than a perl/xs issue
02:55:27<Sysaxed> maybe I should just throw the whole log from here so that he
          can feel the pain?
02:55:41<Sysaxed> :)
@tm604

This comment has been minimized.

Show comment
Hide comment
@tm604

tm604 Apr 18, 2015

On 5.10.1, $x =~ s/\0+$//; fails to match (and thus leaves $x unchanged) when $x is a PV containing null characters which is not itself null-terminated. -Mre=debug reports that the match failure is due to EOL, yet EOS match does work (i.e. when using \z).

5.20.2 does not exhibit this issue.

I don't think it's anything to do with AES or how the input data is generated, just that the ->decrypt method is returning an SV which trips up s//, presumably due to the latter expecting null-termination.

A test case would probably involve just a simple piece of XS along the lines of:

STRLEN size = 5;
RETVAL = NEWSV(0, size);
SvPOK_only(RETVAL);
SvCUR_set(RETVAL, size);
memcpy((UINT8 *) SvPV_nolen(RETVAL), "\0\0\0\0\0", size);

key points being that the SV needs to contain NULs, and not be null-terminated. Devel::Peek should thus show "\0\0\0\0\0" instead of "\0\0\0\0\0"\0 and s/\0+$//; will leave the scalar unchanged, with -Mre=debug reporting match failure on EOL.

tm604 commented Apr 18, 2015

On 5.10.1, $x =~ s/\0+$//; fails to match (and thus leaves $x unchanged) when $x is a PV containing null characters which is not itself null-terminated. -Mre=debug reports that the match failure is due to EOL, yet EOS match does work (i.e. when using \z).

5.20.2 does not exhibit this issue.

I don't think it's anything to do with AES or how the input data is generated, just that the ->decrypt method is returning an SV which trips up s//, presumably due to the latter expecting null-termination.

A test case would probably involve just a simple piece of XS along the lines of:

STRLEN size = 5;
RETVAL = NEWSV(0, size);
SvPOK_only(RETVAL);
SvCUR_set(RETVAL, size);
memcpy((UINT8 *) SvPV_nolen(RETVAL), "\0\0\0\0\0", size);

key points being that the SV needs to contain NULs, and not be null-terminated. Devel::Peek should thus show "\0\0\0\0\0" instead of "\0\0\0\0\0"\0 and s/\0+$//; will leave the scalar unchanged, with -Mre=debug reporting match failure on EOL.

@briandfoy

This comment has been minimized.

Show comment
Hide comment
@briandfoy

briandfoy Apr 18, 2015

Contributor

After some research, I think Crypt::Rijndael is showing correct behavior.
You get back the exact string that was the input. Beyond that, you have to
decide for yourself how to handle the padding.

Contributor

briandfoy commented Apr 18, 2015

After some research, I think Crypt::Rijndael is showing correct behavior.
You get back the exact string that was the input. Beyond that, you have to
decide for yourself how to handle the padding.

@briandfoy briandfoy closed this Apr 18, 2015

@briandfoy briandfoy reopened this Apr 18, 2015

@briandfoy

This comment has been minimized.

Show comment
Hide comment
@briandfoy

briandfoy Apr 18, 2015

Contributor

If there's a problem with how the XS creates the scalar, though, someone will have to help me with that.

Contributor

briandfoy commented Apr 18, 2015

If there's a problem with how the XS creates the scalar, though, someone will have to help me with that.

@tm604

This comment has been minimized.

Show comment
Hide comment
@tm604

tm604 Apr 18, 2015

Sure - which parts do you need help with?

tm604 commented Apr 18, 2015

Sure - which parts do you need help with?

@briandfoy

This comment has been minimized.

Show comment
Hide comment
@briandfoy

briandfoy Apr 27, 2015

Contributor

After looking into this, I haven't been able to reproduce the problem. On 5.10.1 or 5.20.1 I don't have a problem trimming the string with either $ or \z. I think for this to move forward I'd need to know the particulars of the machines or platforms that exhibit the problem.

There still might be an issue with a trailing null, but the problem isn't with the null padding I don't think. I also looked at things with printable characters for padding.

That the original poster has the same problem with a string constructed without the module (01:12:34 in the chat transcript) seems to point to other things going on.

Contributor

briandfoy commented Apr 27, 2015

After looking into this, I haven't been able to reproduce the problem. On 5.10.1 or 5.20.1 I don't have a problem trimming the string with either $ or \z. I think for this to move forward I'd need to know the particulars of the machines or platforms that exhibit the problem.

There still might be an issue with a trailing null, but the problem isn't with the null padding I don't think. I also looked at things with printable characters for padding.

That the original poster has the same problem with a string constructed without the module (01:12:34 in the chat transcript) seems to point to other things going on.

@briandfoy briandfoy closed this Apr 27, 2015

@AlexDaniel

This comment has been minimized.

Show comment
Hide comment
@AlexDaniel

AlexDaniel Apr 27, 2015

And the next message clearly points out that the bug is actually with Crypt::Rijndael (01:28:05 in the chat transcript).
The bug was reproduced by one more person, so I don't see any reason to close this issue. Obviously, the bug is there.
If you need more information -- alright, what do you want to know exactly? But not that closing this issue will somehow bring more info, uh.

AlexDaniel commented Apr 27, 2015

And the next message clearly points out that the bug is actually with Crypt::Rijndael (01:28:05 in the chat transcript).
The bug was reproduced by one more person, so I don't see any reason to close this issue. Obviously, the bug is there.
If you need more information -- alright, what do you want to know exactly? But not that closing this issue will somehow bring more info, uh.

@tm604

This comment has been minimized.

Show comment
Hide comment
@tm604

tm604 Apr 28, 2015

01:12:34 refers to the scalar returned by Crypt::Rijndael. The string was constructed with the module.

Session showing bug being reproduced in perl-5.10.1:

https://gist.github.com/tm604/c222fa5e31643f39f1c9

Again, I believe the issue is due to the absence of the trailing \0 on the SVPV returned by the ->decrypt method.

From perldoc perlguts:

   In the unlikely case of a SV requiring more complex initialisation, you can create an empty SV with newSV(len).  If "len" is 0 an empty SV of type NULL is returned, else an SV of
   type PV is returned with len + 1 (for the NUL) bytes of storage allocated, accessible via SvPVX.  In both cases the SV has value undef.

       SV *sv = newSV(0);   /* no storage allocated  */
       SV *sv = newSV(10);  /* 10 (+1) bytes of uninitialised storage allocated  */

Note the len+1 (for the NULL).

tm604 commented Apr 28, 2015

01:12:34 refers to the scalar returned by Crypt::Rijndael. The string was constructed with the module.

Session showing bug being reproduced in perl-5.10.1:

https://gist.github.com/tm604/c222fa5e31643f39f1c9

Again, I believe the issue is due to the absence of the trailing \0 on the SVPV returned by the ->decrypt method.

From perldoc perlguts:

   In the unlikely case of a SV requiring more complex initialisation, you can create an empty SV with newSV(len).  If "len" is 0 an empty SV of type NULL is returned, else an SV of
   type PV is returned with len + 1 (for the NUL) bytes of storage allocated, accessible via SvPVX.  In both cases the SV has value undef.

       SV *sv = newSV(0);   /* no storage allocated  */
       SV *sv = newSV(10);  /* 10 (+1) bytes of uninitialised storage allocated  */

Note the len+1 (for the NULL).

@Leont

This comment has been minimized.

Show comment
Hide comment
@Leont

Leont Apr 29, 2015

Owner

The regexp engine used to have off-by-one issues on strings that lacked a terminating null-byte, this all sounds very plausible to me. Adding it is the sensible thing IMO.

Owner

Leont commented Apr 29, 2015

The regexp engine used to have off-by-one issues on strings that lacked a terminating null-byte, this all sounds very plausible to me. Adding it is the sensible thing IMO.

@Leont

This comment has been minimized.

Show comment
Hide comment
@Leont

Leont May 23, 2015

Owner

This is solved in 1.13 :-)

Owner

Leont commented May 23, 2015

This is solved in 1.13 :-)

@tm604

This comment has been minimized.

Show comment
Hide comment
@tm604

tm604 May 24, 2015

Much cleaner than my patch, and the original test case now passes for me. Nice work - thanks leont!

tm604 commented May 24, 2015

Much cleaner than my patch, and the original test case now passes for me. Nice work - thanks leont!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment