CS50 Caesar Cipher Bug

Question

Okay, so I am completely stumped. I cannot understand why this programs output acts as if there is a random key everytime.

This program:

#include <cs50.h>
#include <stdio.h>
#include <ctype.h>
#include <string.h>
#include <stdlib.h>

int main(int argc, string argv[])
{
    string sKey = argv[1];
// Make sure program was run with just one command-line argument
    if (argc != 2 || atoi(argv[1]) < 0)
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }

//Counts length of string and checks if all chars are digits
    int counter = 0;
    for (int i = 0; i < strlen(sKey); i++)
    {
        if isdigit(sKey[i])
        {
            counter++;
        }
    }

//Checks if the key is a number
    if (counter != strlen(sKey))
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }

// Convert argv[1] from a `string` to an `int`
    int key = (int)sKey;

// Prompt user for plaintext
    string plaintext = get_string("Plaintext:  ");
    printf("Ciphertext: ");
    for (int i = 0; i < strlen(plaintext); i++)
    {
        if (isalpha(plaintext[i]) && isupper(plaintext[i]))
        {
            printf("%c", (((plaintext[i] - 'A') + key) % 26) + 'A');
        }
        else if (isalpha(plaintext[i]) && islower(plaintext[i]))
        {
            printf("%c", (((plaintext[i] - 'a') + key) % 26) + 'a');
        }
        else
        {
            printf("%c", plaintext[i]);
        }
    }
    printf("\n");
}

Will output this:

caesar/ $ ./caesar 1
Plaintext:  Hello, I'm Justin.
Ciphertext: Fcjjm, G'k Hsqrgl.
caesar/ $ ./caesar 1
Plaintext:  Hello, I'm Justin.
Ciphertext: Pmttw, Q'u Rcabqv.

It seems to be due to the modulo operator, because when I isolated it I could recreate the issue. Is it one of my included libraries? I solved the problem on my own and ended up looking up a solution on youtube only to find my solution performed the same operations as the correct solution. I must be missing something.

Thank you

Wow, totally fixed it thank you. Why would that cause a random key every time? — jlietz93, Feb 26 '22 at 02:20
The real answer to "why it is different every time" is not exactly what @SparKot said but https://en.wikipedia.org/wiki/Address_space_layout_randomization. Without ASLR result would be the same every time the program starts as the base address and all operations leading to the cast of the pointer are the same which would result in identical value on every re-run. ASLR changes base address leading to new value on every re-run. — Alexei Levenkov, Feb 26 '22 at 02:24
Thank you Alexei, I was vaguely wondering if I was stuck in the memory somewhere but I'm obviously a beginner and had no idea were to start Good information to know going forward — jlietz93, Feb 26 '22 at 02:29

score 1 · Accepted Answer · answered Feb 26 '22 at 02:26

This is because int key = (int)sKey; does NOT convert the string to an integer... at least not in the way you think it does. It takes the string pointer sKey (a memory address) to an integer. Since every time you run the progra this can be a different address, this is why it looks random. The correct way to convert a numerical string to a value is using atoi or strtol. The first part of your program should be something like this:

#include <cs50.h>
#include <stdio.h>
#include <ctype.h>
#include <string.h>
#include <stdlib.h>

int main(int argc, string argv[])
{
    string sKey = argv[1];
    int i;

    // Make sure program was run with just one command-line argument
    if (argc != 2)
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }

    // Checks if all chars are digits
    for (int i = 0; sKey[i]; i++)
        if (!isdigit(sKey[i]) break;

    // If the key contains any non-digits, error
    if (sKey[i])
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }

    // Convert argv[1] from a `string` to an `int`
    int key = atoi(sKey);

// The rest should be fine

Thanks a lot, I was able to check my solution after fixing the conversion to int and everything checked out! — jlietz93, Feb 26 '22 at 02:35
Also fixed the logic at the top like you described here, but I see how you simplified the Checks if all chars are digits and if key contains any non-digits, I'll remember that for the future, thank you — jlietz93, Feb 26 '22 at 02:37

CS50 Caesar Cipher Bug

1 Answers1