perlpunk / YAML-PP-p5

A YAML 1.2 processor in perl
https://metacpan.org/pod/YAML::PP
23 stars 8 forks source link

YAML::PP::Load loops infinitely when given tainted string on perl < 5.14 #49

Open haarg opened 11 months ago

haarg commented 11 months ago

There is a bug in perl versions below 5.14 which causes pos to not work correctly with tainted strings. This was fixed by https://github.com/Perl/perl5/commit/fd69380d5d5b95ef16e2521cf4251b34ee0ce151.

YAML::PP reads from strings using YAML::PP::Reader, which relies on pos to keep track of where it is in the string. If pos is not maintained properly, it will loop infinitely.

Rather than using pos, it should be possible to consume the string line by line:

diff --git c/lib/YAML/PP/Reader.pm i/lib/YAML/PP/Reader.pm
index 456630f..aeda5df 100644
--- c/lib/YAML/PP/Reader.pm
+++ i/lib/YAML/PP/Reader.pm
@@ -18,8 +18,7 @@ sub new {

 sub read {
     my ($self) = @_;
-    my $pos = pos $self->{input} || 0;
-    my $yaml = substr($self->{input}, $pos);
+    my $yaml = $self->{input};
     $self->{input} = '';
     return $yaml;
 }
@@ -29,7 +28,7 @@ sub readline {
     unless (length $self->{input}) {
         return;
     }
-    if ( $self->{input} =~ m/\G([^\r\n]*(?:\n|\r\n|\r|\z))/g ) {
+    if ( $self->{input} =~ s/\A([^\r\n]*(?:\n|\r\n|\r|\z))// ) {
         my $line = $1;
         unless (length $line) {
             $self->{input} = '';

This could lead to copying large strings though. It may also be reasonable to untaint the string before processing it.

perlpunk commented 11 months ago

Maybe I could make the behaviour depending on the perl version. Or maybe I could just check if the string is tainted?