From bb2e32141d7d6f810dbf22fa9cbc211e9e67f88b Mon Sep 17 00:00:00 2001 From: Doug Bell Date: Tue, 10 Aug 2010 21:17:20 -0500 Subject: [PATCH 1/5] fix 11773 Pluggable allows arbitrary module load --- docs/changelog/7.x.x.txt | 1 + lib/WebGUI/Pluggable.pm | 5 +++++ t/Pluggable.t | 10 +++++++++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 25895da56..d81bcf4fb 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -14,6 +14,7 @@ - fixed #11772: Metadata in Post doesn't set default value correctly - fixed #11768: Edit Branch does not update File wgaccess permissions - added Asset Report Asset allowing creation of reports based on Asset Properties. + - fixed #11773: Pluggable allows arbitrary module loading 7.9.10 - fixed #11721: spamStopWords not in WebGUI.conf.original diff --git a/lib/WebGUI/Pluggable.pm b/lib/WebGUI/Pluggable.pm index c5608288c..b9acebf29 100644 --- a/lib/WebGUI/Pluggable.pm +++ b/lib/WebGUI/Pluggable.pm @@ -232,6 +232,11 @@ sub load { croak "Could not load $module because $moduleError{$module}"; } + # Sanitize + if ( $module !~ m{^\w+(?:\w+|::)*\w+$} ) { + croak "Invalid module name: $module"; + } + # Try to load the module my $modulePath = $module . ".pm"; $modulePath =~ s{::|'}{/}g; diff --git a/t/Pluggable.t b/t/Pluggable.t index 6082ef979..905ba16eb 100644 --- a/t/Pluggable.t +++ b/t/Pluggable.t @@ -41,7 +41,7 @@ use WebGUI::Pluggable; #---------------------------------------------------------------------------- # Tests -plan tests => 12; # Increment this number for each test you create +plan tests => 18; # Increment this number for each test you create #---------------------------------------------------------------------------- # put your tests here @@ -62,6 +62,14 @@ is($dumper->Dump, q|$VAR1 = { }; |, "Can instanciate an object."); +ok( !eval{ WebGUI::Pluggable::load( '::HA::HA' ); 1 }, 'load dies on bad input' ); +like( $@, qr/^\QInvalid module name: ::HA::HA/, 'helpful error message' ); + +ok( !eval{ WebGUI::Pluggable::load( 'HA::HA::' ); 1 }, 'load dies on bad input' ); +ok( !eval{ WebGUI::Pluggable::load( 'HA::..::..::HA' ); 1 }, 'load dies on bad input' ); +ok( !eval{ WebGUI::Pluggable::load( '..::..::..::HA' ); 1 }, 'load dies on bad input' ); +ok( !eval{ WebGUI::Pluggable::load( 'uploads::ik::jo::ikjosdfwefsdfsefwef::myfile.txt\0.pm' ); 1 }, 'load dies on bad input' ); + #---------------------------------------------------------------------------- # Test find and findAndLoad { # Block to localize @INC From e71ce095883554363dacb466cd2ce5b96951a72e Mon Sep 17 00:00:00 2001 From: Doug Bell Date: Tue, 10 Aug 2010 21:18:22 -0500 Subject: [PATCH 2/5] add "pm" to list of verboten uploads --- lib/WebGUI/Storage.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/WebGUI/Storage.pm b/lib/WebGUI/Storage.pm index 91a5e0f84..c24b51aaa 100644 --- a/lib/WebGUI/Storage.pm +++ b/lib/WebGUI/Storage.pm @@ -315,7 +315,7 @@ sub addFileFromFilesystem { return undef; } my $filename = (File::Spec->splitpath( $pathToFile ))[2]; - if (isIn($self->getFileExtension($filename), qw(pl perl sh cgi php asp))) { + if (isIn($self->getFileExtension($filename), qw(pl perl sh cgi php asp pm))) { $filename =~ s/\./\_/g; $filename .= ".txt"; } From da4f2f08b37dbb2c48cbe4ea6cc2f0007dc9c548 Mon Sep 17 00:00:00 2001 From: Doug Bell Date: Tue, 10 Aug 2010 21:20:19 -0500 Subject: [PATCH 3/5] graham's regex makes more sense than mine --- lib/WebGUI/Pluggable.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/WebGUI/Pluggable.pm b/lib/WebGUI/Pluggable.pm index b9acebf29..552c1175f 100644 --- a/lib/WebGUI/Pluggable.pm +++ b/lib/WebGUI/Pluggable.pm @@ -233,7 +233,7 @@ sub load { } # Sanitize - if ( $module !~ m{^\w+(?:\w+|::)*\w+$} ) { + if ( $module !~ m{^\w+(?:::\w+)*$} ) { croak "Invalid module name: $module"; } From 7861679aee7e6cb334fd49f453550332f22bf37a Mon Sep 17 00:00:00 2001 From: Doug Bell Date: Tue, 10 Aug 2010 21:23:12 -0500 Subject: [PATCH 4/5] add a test for :::: --- t/Pluggable.t | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/Pluggable.t b/t/Pluggable.t index 905ba16eb..972856121 100644 --- a/t/Pluggable.t +++ b/t/Pluggable.t @@ -41,7 +41,7 @@ use WebGUI::Pluggable; #---------------------------------------------------------------------------- # Tests -plan tests => 18; # Increment this number for each test you create +plan tests => 19; # Increment this number for each test you create #---------------------------------------------------------------------------- # put your tests here @@ -69,6 +69,7 @@ ok( !eval{ WebGUI::Pluggable::load( 'HA::HA::' ); 1 }, 'load dies on bad input' ok( !eval{ WebGUI::Pluggable::load( 'HA::..::..::HA' ); 1 }, 'load dies on bad input' ); ok( !eval{ WebGUI::Pluggable::load( '..::..::..::HA' ); 1 }, 'load dies on bad input' ); ok( !eval{ WebGUI::Pluggable::load( 'uploads::ik::jo::ikjosdfwefsdfsefwef::myfile.txt\0.pm' ); 1 }, 'load dies on bad input' ); +ok( !eval{ WebGUI::Pluggable::load( 'HA::::HA' ); 1 }, 'load dies on bad input' ); #---------------------------------------------------------------------------- # Test find and findAndLoad From 50d81eb007888d7a414bc6644fe2c0ae238cc068 Mon Sep 17 00:00:00 2001 From: Doug Bell Date: Tue, 10 Aug 2010 21:25:18 -0500 Subject: [PATCH 5/5] use Test::Exception instead --- t/Pluggable.t | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/t/Pluggable.t b/t/Pluggable.t index 972856121..257b23b2f 100644 --- a/t/Pluggable.t +++ b/t/Pluggable.t @@ -31,6 +31,7 @@ use Test::Deep::Shallow; use Test::Deep::Blessed; use Test::Deep::Isa; use Test::Deep::Set; +use Test::Exception; use WebGUI::Pluggable; @@ -62,14 +63,14 @@ is($dumper->Dump, q|$VAR1 = { }; |, "Can instanciate an object."); -ok( !eval{ WebGUI::Pluggable::load( '::HA::HA' ); 1 }, 'load dies on bad input' ); +dies_ok { WebGUI::Pluggable::load( '::HA::HA' ) } 'load dies on bad input'; like( $@, qr/^\QInvalid module name: ::HA::HA/, 'helpful error message' ); -ok( !eval{ WebGUI::Pluggable::load( 'HA::HA::' ); 1 }, 'load dies on bad input' ); -ok( !eval{ WebGUI::Pluggable::load( 'HA::..::..::HA' ); 1 }, 'load dies on bad input' ); -ok( !eval{ WebGUI::Pluggable::load( '..::..::..::HA' ); 1 }, 'load dies on bad input' ); -ok( !eval{ WebGUI::Pluggable::load( 'uploads::ik::jo::ikjosdfwefsdfsefwef::myfile.txt\0.pm' ); 1 }, 'load dies on bad input' ); -ok( !eval{ WebGUI::Pluggable::load( 'HA::::HA' ); 1 }, 'load dies on bad input' ); +dies_ok { WebGUI::Pluggable::load( 'HA::HA::' ) } 'load dies on bad input'; +dies_ok { WebGUI::Pluggable::load( 'HA::..::..::HA' ) } 'load dies on bad input'; +dies_ok { WebGUI::Pluggable::load( '..::..::..::HA' ) } 'load dies on bad input'; +dies_ok { WebGUI::Pluggable::load( 'uploads::ik::jo::ikjosdfwefsdfsefwef::myfile.txt\0.pm' ) } 'load dies on bad input'; +dies_ok { WebGUI::Pluggable::load( 'HA::::HA' ) } 'load dies on bad input'; #---------------------------------------------------------------------------- # Test find and findAndLoad