Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am trying to learn C and I came across the ROT13 scrambling system used to store some passwords.

Assuming the user types everything in correctly (uses 1 argument, uses a string not an int, etc.) would this be correct/safe to use? Or is there anything at all that I am doing wrong you can point out to me (techniques, indentation, anything at all)?

#include <stdio.h>
#include <string.h>

int main(int argc, char** argv)
{
    char* word = argv[1];
    int key = 13;

    // all the letters in the first argument 
    for (int n = 0, len = strlen(word); n < len; n++)
    {
        int currentLetter = word[n];

        char cipher = currentLetter + key;

        // make sure the next letter isn't over 26 or it isn't a ascii letter
        // if it is, do %26
        if ((currentLetter - 'a') + key > 26)
        {
            key = (currentLetter - 'a') + key) % 26;
            cipher = 'a' + key;
        }

        printf("%c", cipher);
        // reset the key and do the next letter
        key = 13;
        }
    }
    printf("\n");
    return 0;
}
share|improve this question

1 Answer 1

up vote 2 down vote accepted

Test your program with a variety of inputs. Include "abcdefghijklmnopqrstuvwxyz".

Have a look what happens for 'n'. Always try to make sure you test low values, high values and edge cases. 'Wrapping round' is a classic edge case.

Then look at this line:

if ((currentLetter - 'a') + key > 26)

This program only works for lower case 'passwords'.

You might look at the functions in #include <ctype.h> such as tolower(.) or islower(.) to extend to mixing upper and lower case.

NB Obviously ROT13 has no real cryptographic value of any kind. I mention that because I have seen it (and things not better) used in applications. You're doing this as a learning exercise. Right?

share|improve this answer
    
IMO ROT13 is perfectly fine to hide simple text from plain text searches in a fast and reliable way. You don't always have to use SHA-1 or AES encryption. It's not about the algorithm or it's security, it's about the actual use case. –  Mario Feb 19 at 7:50
    
@mario. A program that searches for 'plain text' will identify ROT13 strings as plain-text. The OP talks about passwords. The use case is that someone obtains the password file and seeks to hack it. Unless they're in kindergarten any self respecting hacker can pretty much read ROT13 let alone hack it. It might be fun to play games but I stand by my comment. No cryptographic value. –  Dan Allen Feb 19 at 7:58
1  
Okay, right, missed the password part. It's definitely not a good idea for passwords. :) –  Mario Feb 19 at 8:06
    
@Dan Allen Yeah, I this was used for just practice. And actually even before I used 'toLower(currentLetter)'. It did not work correctly. For example when I do './rot32 NICHOLAS' it outputs to '[VPU\YN`' When it should only output letters... –  Synapz Feb 19 at 13:47
    
@Synapz I thought it was practice. But I really have seen it used in real systems protecting real billions of real money. You might simplify with if(cipher>'z'){cipher=cipher-26;} –  Dan Allen Feb 19 at 13:57

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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