Skip to content

Commit

Permalink
Add option to scrub html from secret
Browse files Browse the repository at this point in the history
This new option allows admins to enable or disable scrubbing html
from secrets on view and store.
  • Loading branch information
renderorange committed May 7, 2024
1 parent e9dcf6e commit d7141bd
Show file tree
Hide file tree
Showing 16 changed files with 211 additions and 12 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
sudo apt-get install cpanminus sqlite3
- name: Install perl dependencies
run: |
sudo cpanm -n Config::Tiny Crypt::Eksblowfish::Bcrypt Crypt::Random Cwd Dancer2 Dancer2::Session::Cookie Data::Structure::Util DBD::SQLite DBI Digest::SHA Encode Getopt::Long HTTP::Status Moo MooX::ClassAttribute namespace::clean Plack::Builder Plack::Middleware::TrailingSlashKiller Pod::Usage Scalar::Util Session::Storage::Secure strictures Template::Toolkit Time::Piece Try::Tiny
sudo cpanm -n Config::Tiny Crypt::Eksblowfish::Bcrypt Crypt::Random Cwd Dancer2 Dancer2::Session::Cookie Data::Structure::Util DBD::SQLite DBI Digest::SHA Encode Getopt::Long HTML::Strip HTTP::Status Moo MooX::ClassAttribute namespace::clean Plack::Builder Plack::Middleware::TrailingSlashKiller Pod::Usage Scalar::Util Session::Storage::Secure strictures Template::Toolkit Time::Piece Try::Tiny
- name: Install perl test dependencies
run: |
# Test::More and File::Spec are provided by perl-modules-*
Expand Down
9 changes: 6 additions & 3 deletions INSTALLATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ These instructions walk through a basic installation of the Pasteburn applicatio

The Perl dependencies for this project are listed in the `cpanfile` within the repo.

cpanm -n Config::Tiny Crypt::Eksblowfish::Bcrypt Crypt::Random Cwd Dancer2 Dancer2::Session::Cookie Data::Structure::Util DBD::SQLite DBI Digest::SHA Encode Getopt::Long HTTP::Status Moo MooX::ClassAttribute namespace::clean Plack::Builder Plack::Middleware::TrailingSlashKiller Pod::Usage Scalar::Util Session::Storage::Secure strictures Template::Toolkit Time::Piece Try::Tiny
cpanm -n Config::Tiny Crypt::Eksblowfish::Bcrypt Crypt::Random Cwd Dancer2 Dancer2::Session::Cookie Data::Structure::Util DBD::SQLite DBI Digest::SHA Encode Getopt::Long HTML::Strip HTTP::Status Moo MooX::ClassAttribute namespace::clean Plack::Builder Plack::Middleware::TrailingSlashKiller Pod::Usage Scalar::Util Session::Storage::Secure strictures Template::Toolkit Time::Piece Try::Tiny

## CREATE THE DATABASE

Expand All @@ -50,12 +50,15 @@ After creating the file, edit and update the values accordingly.

- secret

The `secret` section key is required, and `age` option key within it.
The `secret` section key is required, `age` and `scrub` option keys within it.

[secret]
age = 604800
scrub = 1

To change the default time to expire secrets, change the `age` value. The value must be a positive integer. The `age` value is only enforced if running the `delete_expired_secrets.pl` script, as noted below.
If `scrub` is set to 1, HTML tags will be removed from the secret string before storing and again as it's retrieved from the database. If set to 0, HTML tags will not be removed from the secret.

NOTE: Setting `scrub` to 0 means XSS vulnerabilities will be possible in the textarea box as it's displayed. Disable this setting with caution.

- passphrase

Expand Down
1 change: 1 addition & 0 deletions cpanfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ requires 'DBI';
requires 'Digest::SHA';
requires 'Encode';
requires 'Getopt::Long';
requires 'HTML::Strip';
requires 'HTTP::Status';
requires 'Moo';
requires 'MooX::ClassAttribute';
Expand Down
1 change: 1 addition & 0 deletions examples/config.ini.example
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[secret]
age = 604800
scrub = 1
[passphrase]
allow_blank = 0
[cookie]
Expand Down
8 changes: 7 additions & 1 deletion lib/Pasteburn.pm
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ BEGIN {
die("FATAL: session Cookie secret_key is not set");
}

set secret => $conf->{secret};
set passphrase => $conf->{passphrase};

set views => config->{appdir} . 'views';
Expand Down Expand Up @@ -123,13 +124,18 @@ B<NOTE:> If the C<$ENV{HOME}/.config/pasteburn/> directory exists, C<config.ini>
=item secret
The C<secret> section key is required, and C<age> option key within it.
The C<secret> section key is required, C<age> and C<scrub> option keys within it.
[secret]
age = 604800
scrub = 1
To change the default time to expire secrets, change the C<age> value. The value must be a positive integer. The C<age> value is only enforced if running the C<delete_expired_secrets.pl> script, as noted below.
If C<scrub> is set to 1, HTML tags will be removed from the secret string before storing and again as it's retrieved from the database. If set to 0, HTML tags will not be removed from the secret.
B<NOTE:> Setting C<scrub> to 0 means XSS vulnerabilities will be possible in the textarea box as it's displayed. Disable this setting with caution.
=item passphrase
The C<passphrase> section key is required, and the C<allow_blank> option key within it.
Expand Down
12 changes: 11 additions & 1 deletion lib/Pasteburn/Config.pm
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ sub _validate {
die "config section secret age must be a positive integer\n";
}

# verify secret scrub exists
unless ( exists $config->{secret}{scrub} ) {
die "config section secret scrub is required\n";
}

# verify passphrase allow_blank exists
unless ( exists $config->{passphrase}{allow_blank} ) {
die "config section passphrase allow_blank is required\n";
Expand Down Expand Up @@ -137,13 +142,18 @@ B<NOTE:> If the C<$ENV{HOME}/.config/pasteburn/> directory exists, C<config.ini>
=item secret
The C<secret> section key is required, and C<age> option key within it.
The C<secret> section key is required, C<age> and C<scrub> option keys within it.
[secret]
age = 604800
scrub = 1
To change the default time to expire secrets, change the C<age> value. The value must be a positive integer. The C<age> value is only enforced if running the C<delete_expired_secrets.pl> script, as noted below.
If C<scrub> is set to 1, HTML tags will be removed from the secret string before storing and again as it's retrieved from the database. If set to 0, HTML tags will not be removed from the secret.
B<NOTE:> Setting C<scrub> to 0 means XSS vulnerabilities will be possible in the textarea box as it's displayed. Disable this setting with caution.
=item passphrase
The C<passphrase> section key is required, and the C<allow_blank> option key within it.
Expand Down
4 changes: 2 additions & 2 deletions lib/Pasteburn/Controller/Secret.pm
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ post q{/secret} => sub {
}

my $secret_obj = Pasteburn::Model::Secrets->new( secret => $secret, passphrase => $passphrase );
$secret_obj->store;
$secret_obj->store( scrub => config->{secret}{scrub} );

my $session_secrets = session->read('secrets');
$session_secrets->{ $secret_obj->id }{runmode} = 'new';
Expand Down Expand Up @@ -184,7 +184,7 @@ post q{/secret/:id} => sub {
return template secret => $template_params;
}

my $decoded_secret = $secret_obj->decode_secret( passphrase => $passphrase );
my $decoded_secret = $secret_obj->decode_secret( passphrase => $passphrase, scrub => config->{secret}{scrub} );
if ($decoded_secret) {

# this will not delete from the author session unless they also view it.
Expand Down
33 changes: 30 additions & 3 deletions lib/Pasteburn/Model/Secrets.pm
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use Pasteburn::Crypt::Hash ();
use Pasteburn::Crypt::Storage ();
use Crypt::Random ();
use Digest::SHA ();
use HTML::Strip;

use Moo;
use MooX::ClassAttribute;
Expand Down Expand Up @@ -111,6 +112,10 @@ sub get {

sub store {
my $self = shift;
my $arg = {
scrub => undef,
@_,
};

foreach my $attribute ( 'passphrase', 'secret' ) {
unless ( defined $self->{$attribute} ) {
Expand All @@ -123,6 +128,13 @@ sub store {
my $crypt_hash = Pasteburn::Crypt::Hash->new();
my $hashed_passphrase = $crypt_hash->generate( string => $self->passphrase );

# strip html tags from secret before storing
if ( $arg->{scrub} ) {
my $html_strip = HTML::Strip->new();
$self->_set_secret( $html_strip->parse( $self->secret ) );
$html_strip->eof;
}

my $crypt_storage = Pasteburn::Crypt::Storage->new( passphrase => $self->passphrase );
my $encoded_secret = $crypt_storage->encode( secret => $self->secret );

Expand All @@ -147,6 +159,8 @@ sub store {
# set id into the object so we can _update_object using it as a select value.
$self->_set_id($id);

# always update the object with the stored data from the database.
# we don't want the decoded secret or passphrase in the object.
$self->_update_object;

return $result;
Expand Down Expand Up @@ -222,6 +236,7 @@ sub decode_secret {
my $self = shift;
my $arg = {
passphrase => undef,
scrub => undef,
@_,
};

Expand All @@ -237,8 +252,16 @@ sub decode_secret {
die "passphrase is required";
}

my $crypt_storage = Pasteburn::Crypt::Storage->new( passphrase => $arg->{passphrase} );
return $crypt_storage->decode( secret => $self->secret );
my $crypt_storage = Pasteburn::Crypt::Storage->new( passphrase => $arg->{passphrase} );
my $decoded_secret = $crypt_storage->decode( secret => $self->secret );

if ( $arg->{scrub} ) {
my $html_strip = HTML::Strip->new();
$decoded_secret = $html_strip->parse($decoded_secret);
$html_strip->eof;
}

return $decoded_secret;
}

sub delete_secret {
Expand Down Expand Up @@ -288,7 +311,7 @@ Pasteburn::Model::Secrets - data access layer for Secrets
my $secret_obj = Pasteburn::Model::Secrets->get( id => $id );
$secret_obj->validate_passphrase( passphrase => $passphrase );
my $decoded_secret = $secret_obj->decode_secret( passphrase => $passphrase );
my $decoded_secret = $secret_obj->decode_secret( passphrase => $passphrase, scrub => 1 );
$secret_obj->delete_secret;
Expand Down Expand Up @@ -402,6 +425,10 @@ Object method to decode the stored secret using the submitted passphrase.
=item passphrase
=item scrub
Truthy value indicating whether the secret string should have the HTML tags removed before returning.
=back
=head3 RETURNS
Expand Down
39 changes: 39 additions & 0 deletions t/integration/backend/create-scrub-new-secret.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
use strict;
use warnings;

use FindBin ();
use lib "$FindBin::RealBin/../../lib", "$FindBin::RealBin/../../../lib";
use Pasteburn::Test;
use Pasteburn::Model::Secrets;

SCRUB_SECRET_ENABLED: {
note( 'scrub secret enabled' );
my $secret_non_html = 'blaine was here text';
my $secret = '</textarea><script>blaine was here script</script>' . $secret_non_html;
my $passphrase = 'mypassphrase';
my $secret_obj = Pasteburn::Model::Secrets->new( secret => $secret, passphrase => $passphrase );

ok( $secret_obj->store( scrub => 1 ), 'store was successful' );

# here we're intentionally not scrubbing the outgoing secret because we need to ensure
# scrubbing the secret inbound works correctly.
my $decoded_secret = $secret_obj->decode_secret( passphrase => $passphrase );
is( $decoded_secret, $secret_non_html, 'returned secret only contains the non-html string' )
}

SCRUB_SECRET_DISABLED: {
note( 'scrub secret disabled' );
my $secret_non_html = 'blaine was here text';
my $secret = '</textarea><script>blaine was here script</script>' . $secret_non_html;
my $passphrase = 'mypassphrase';
my $secret_obj = Pasteburn::Model::Secrets->new( secret => $secret, passphrase => $passphrase );

ok( $secret_obj->store( scrub => 0 ), 'store was successful' );

# here we're intentionally not scrubbing the outgoing secret because we need to ensure
# scrubbing the secret inbound works correctly.
my $decoded_secret = $secret_obj->decode_secret( passphrase => $passphrase );
is( $decoded_secret, $secret, 'returned secret contains the html and non-html string parts' );
}

done_testing;
40 changes: 40 additions & 0 deletions t/integration/backend/decode-scrub-secret.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
use strict;
use warnings;

use FindBin ();
use lib "$FindBin::RealBin/../../lib", "$FindBin::RealBin/../../../lib";
use Pasteburn::Test;
use Pasteburn::Model::Secrets;

SCRUB_SECRET_ENABLED: {
note( 'scrub secret enabled' );
my $secret_non_html = 'blaine was here text';
my $secret = '</textarea><script>blaine was here script</script>' . $secret_non_html;
my $passphrase = 'mypassphrase';
my $secret_obj = Pasteburn::Model::Secrets->new( secret => $secret, passphrase => $passphrase );

# here we're intentionally not scrubbing the incoming secret because we need to ensure
# decoding a secret with html works correctly.
ok( $secret_obj->store, 'stored new secret' );

my $decoded_secret = $secret_obj->decode_secret( passphrase => $passphrase, scrub => 1 );
isnt( $decoded_secret, $secret, "returned secret doesn't match" );
is( $decoded_secret, $secret_non_html, 'returned secret only contains the non-html string' );
}

SCRUB_SECRET_DISABLED: {
note( 'scrub secret disabled' );
my $secret_non_html = 'blaine was here text';
my $secret = '</textarea><script>blaine was here script</script>' . $secret_non_html;
my $passphrase = 'mypassphrase';
my $secret_obj = Pasteburn::Model::Secrets->new( secret => $secret, passphrase => $passphrase );

# here we're intentionally not scrubbing the incoming secret because we need to ensure
# decoding a secret with html works correctly.
ok( $secret_obj->store, 'stored new secret' );

my $decoded_secret = $secret_obj->decode_secret( passphrase => $passphrase, scrub => 0 );
is( $decoded_secret, $secret, 'returned secret contains the html and non-html string parts' );
}

done_testing;
1 change: 1 addition & 0 deletions t/integration/frontend/allow-empty-passphrase.t
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use HTTP::Request ();
my $config = {
secret => {
age => 1,
scrub => 1,
},
passphrase => {
allow_blank => 1,
Expand Down
1 change: 1 addition & 0 deletions t/integration/frontend/create-new-secret.t
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use HTTP::Request ();
my $config = {
secret => {
age => 1,
scrub => 1,
},
passphrase => {
allow_blank => 0,
Expand Down
67 changes: 67 additions & 0 deletions t/integration/frontend/view-scrub-secret.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
use strict;
use warnings;

use FindBin ();
use lib "$FindBin::RealBin/../../lib", "$FindBin::RealBin/../../../lib";
use Pasteburn::Test;

use HTTP::Request ();

my $config = {
secret => {
age => 1,
scrub => 1,
},
passphrase => {
allow_blank => 0,
},
cookie => {
secret_key => 'testing',
},
footer => {
links => 1,
},
};

my $test = Pasteburn::Test::create_test_app(
config => $config,
);

my $method = 'GET';
my $endpoint = '/secret';

my $headers = [];
my $request = HTTP::Request->new( $method, $endpoint, $headers );

my $response = $test->request( $request );

like( $response->content, qr/Create a secret message/, 'secret page is loaded' );

$method = 'POST';
$endpoint = '/secret';
my $passphrase = 'testpassphrase';
my $secret_non_html = 'blaine was here text';
my $secret = '</textarea><script>blaine was here script</script>' . $secret_non_html;

$headers = [ 'Content-Type' => 'application/x-www-form-urlencoded' ];
my $data = 'passphrase=' . $passphrase . '&secret=' . $secret;
$request = HTTP::Request->new( $method, $endpoint, $headers, $data );

note( 'submitted post to create secret' );
$response = $test->request( $request );

my ( $secret_id ) = $response->header('Location') =~ /\/secret\/(\w+)$/;

$endpoint = '/secret/' . $secret_id;
$data = 'passphrase=' . $passphrase;
$request = HTTP::Request->new( $method, $endpoint, $headers, $data );

note( 'submitted post to view secret' );
$response = $test->request( $request );

is( $response->code, 200, 'response code is 200' );

my $regex = 'readonly>' . $secret_non_html . '<\/textarea';
like( $response->content, qr/$regex/, 'response content contains expected decoded secret' );

done_testing;
1 change: 1 addition & 0 deletions t/integration/frontend/view-secret.t
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use HTTP::Request ();
my $config = {
secret => {
age => 1,
scrub => 1,
},
passphrase => {
allow_blank => 0,
Expand Down
1 change: 1 addition & 0 deletions t/unit/lib-Pasteburn-Config/_load.t
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use_ok( $class );
my $config_expected = {
secret => {
age => 1,
scrub => 1,
},
passphrase => {
allow_blank => 0,
Expand Down
Loading

0 comments on commit d7141bd

Please sign in to comment.