8
\$\begingroup\$

I'm trying to improve my Perl and am doing some wargames online. I wrote a small utility to help me pass a stage.

The main weakness of a simple substitution cipher is repeated use of a simple key. In the previous exercise you were able to introduce arbitrary plaintext to expose the key. In this example, the cipher mechanism is not available to you, the attacker.

However, you have been lucky. You have intercepted more than one message. The password to the next level is found in the file ‘krypton4’. You have also found 3 other files. (found1, found2, found3)

You know the following important details

  • The message plaintexts are in English
  • They were produced from the same key
  • The cyphertexts are UPPERCASE
  • The ciphertexts in blocks of 5

#!/usr/bin/perl

use strict;
use warnings;
use feature 'say';


use File::Slurp;


my @FILENAMES = ("found1", "found2", "found3");
my @LETTERS = ("A" .. "Z");
my @ENGLISH_FREQ = ("E", "T", "A", "O", "I", "N", "S", 
                    "H", "R", "D", "L", "C", "U", "M", 
                    "W", "F", "G", "Y", "P", "B", "V", 
                    "K", "J", "X", "Q", "Z");

my $ENCRYPTED_STRING = read_file("krypton4");
$ENCRYPTED_STRING =~ s/\s+//g;


sub count_frequency {
    my ($string) = @_;
    my %occurences = count_occurence($string);
    return map { $_ => $occurences{$_} / length($string) } keys %occurences;
}

sub count_occurence {
    my ($string) = @_;
    my %frequency = map { $_ => 0 } @LETTERS;
    foreach (split //, $string) {
        $frequency{$_}++;
    }
    return %frequency;
}

sub get_single_key {
    my (%text_freq) = @_;
    my @text = ();
    foreach my $name ( sort { $text_freq{$b} <=> $text_freq{$a} } keys %text_freq ) {
        push @text, $name;
    }
    return map { $text[$_] => $ENGLISH_FREQ[$_] } (0 .. $#text);
}

foreach (@FILENAMES) {
    # Read the text
    my $text = read_file($_);
    $text =~ s/\s+//g;

    # Perform Letter Frequency Analysis
    my %cypher_key = get_single_key(count_frequency($text));

    # Try to decrypt the text
    foreach (split //, $ENCRYPTED_STRING) {
        print "$cypher_key{$_}";
    }
    say "";
}

It was some puzzling afterwards, but it did its job.

\$\endgroup\$
  • \$\begingroup\$ You have indeed improved. Well done. But there are some new issues. ;) \$\endgroup\$ – simbabque Nov 18 '17 at 13:30
  • \$\begingroup\$ @simbabque Thank you. Dammit still no perfect Perl :) \$\endgroup\$ – Ludisposed Nov 18 '17 at 13:38
  • \$\begingroup\$ I don't think that exists, and if it does, it will take years. Try to read some source code on CPAN. That's a good way to learn. Not entirely sure where to start there, but as long as it has strict and warnings and a decent number of likes on metacpan chances are you will learn something. There's also the Kwalitee score for modules, and you can use those metrics against your own code. And Perl::Critic of course, which will tell you in your face if you did something stupid. ;) \$\endgroup\$ – simbabque Nov 18 '17 at 13:48
10
\$\begingroup\$
use File::Slurp;

File::Slurp is considered broken by a lot of people. A good alternative is Path::Tiny.

It's also good practice to import only what you need when you use a module. That helps others to see where a function is coming from (like your read_file, which I tried to find in your code), and it helps to keep your namespace clean.

use Path::Tiny 'path';

my $text = path('krypton4')->slurp;

my @FILENAMES = ("found1", "found2", "found3");

All caps names are typically used for constants only. You can create a constant with the use constants pragma. That works by installing a sub into your namespace that returns a scalar. As a function, there's no sigil, and you need to use references for arrays.

The qw() quoted words operator is a nice shrotand for creating lists of quoted words, as you don't have to bother with commas and quotes.

use constant FILENAMES => [ qw(found1 found2 found3) ];

my $ENCRYPTED_STRING = read_file("krypton4");
$ENCRYPTED_STRING =~ s/\s+//g;

You only use this variable once. It's not really configuration either, so making it a constant feels wrong. In Perl, we try to declare variables as close to where they are used as possible, and in the smallest scope possible. I would recommend simply using Path::Tiny's path down in your foreach loop.

You could however add a constant for the krypton4 file name if you wanted.

use constant ENCRYPTED_FILE => 'krypton4';

If your Perl is at least 5.14, you can use the /r modifier on regex substitution to return an altered version, instead of doing in-place edit.

foreach my $item (split //, path(ENCRYPTED_FILE)->slurp =~ s/\s+//gr) { ... }

If your Perl is older, you can also use map to do that in one line. Note that s///g returns the number of substitutions, you have to explicitly return $_ from the block to get the text.

foreach my $item (split //, map { s/\s+//g; $_ } path(ENCRYPTED_FILE)->slurp) { ... }

foreach (split //, $string) {
    $frequency{$_}++;
}

If you use $_ explicitly in a loop body, it's clearer to use a named variable instead. I would call this $letter.

foreach my $letter (split //, $string) {
    $frequency{$letter}++;
}

sub get_single_key {
    my (%text_freq) = @_;
    my @text = ();
    foreach my $name ( sort { $text_freq{$b} <=> $text_freq{$a} } keys %text_freq ) {
        push @text, $name;
    }

    return map { $text[uc($_)] => $ENGLISH_FREQ[$_] } (0 .. $#text);
}

The frequency you've calculated with count_frequency is the frequency of each letter in the text. Your variable %text_frequency should thus be named %letter_frequency. But that's long. Why not stick with %frequency? It worked above.

You don't need to initialize arrays and hashes with an empty list. Perl does that for you.

I didn't read the algorithm description, so I don't know exactly what this does, but @text feels like the name is wrong. That variable is not an array of texts, or ordered pieces of one text. I think it contains the letters ordered by frequency. So @letters might be better.

It's good that you used a named loop variable, but again, this is a letter, right? So $letter is way more fitting than $name.

You can just assign the list of keys that sort returns to @letters.

There's a bug in your map statement. The $_ is not the letter, but the index. Your iteration 0 .. $#text gives you all the indexes. You are ucing the index, not the value.

You don't need the parentheses around the range. map takes a list, and the range operator .. gives you a list.

sub get_single_key {
    my (%frequency) = @_;

    my @letters = sort { $frequency{$b} <=> $frequency{$a} } keys %frequency;

    return map { uc $letters[$_] => ENGLISH_FREQUENCIES->[0] } 0 .. $#letters;
}

Since you have that comment Perform Letter Frequency Analysis where you use this function, and you return a hash, which implies a bunch of things, the name get_single_key might not be optimal.


foreach (split //, $ENCRYPTED_STRING) {
    print "$cypher_key{$_}";
}

A postfix foreach (or for for brevity) will be easier to read here.

You don't have to quote a single variable. There is no need for interpolation.

print $cypher_key{$_} 
    for split //, path(ENCRYPTED_FILE)->slurp =~ s/\s+//gr;

say "";

It's shorter than print "\n" (or print "$/" if you insist on being portable), but it just feels off. It's good though that you realized that you need an empty string so it does not output $_ instead.

\$\endgroup\$
  • 1
    \$\begingroup\$ Great answer yet again. The only thing I don't grasp yet is how those constants are called, but will figure it out. I've edited your code slightly to fix a leftover typo, I hope you don't mind. :) \$\endgroup\$ – Ludisposed Nov 18 '17 at 21:10
  • \$\begingroup\$ @Ludisposed constants get turned into functions at compile time. That's why they are constants. The compiler sees that they contain a fixed value, and folds them. Way faster than variables. \$\endgroup\$ – simbabque Nov 19 '17 at 0:31
3
\$\begingroup\$

I've found a few things (Not Perl specific) that could be improved upon.

  1. I created a new cypher_key for each file. To have better accuracy I should have merged the frequencies for each file to a new hash, for instance total_frequencies. And once all files a processed try to break the text.

  2. My count_frequencies is redundant, I don't do anything with the frequencies. I could have just return the count_occurences to yield the same results.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.