patterncModerate
Checking that a password contains a '$', a digit, and an uppercase letter
Viewed 0 times
uppercasecheckingletterpasswordthatcontainsanddigit
Problem
I have this code, which reads in a string password from the user and checks if it has a dollar sign
Your password is correct
I'm pretty shabby with C and I was just wondering if there was a way to reduce part of the code. I also just want to check if the pointer/array handling is correct, since I've only just started using it properly.
$, a number, and an upper case letter. If it has all three, it prints:Your password is correct
#include
#include
#include
#include
#define MAXSIZE 100
void password_read(char password[]);
int password_check(char password[]);
int
main(int argc, char *argv[]){
char password[MAXSIZE];
int sum;
password_read(password);
sum = password_check(password);
if (sum) {
printf("Your password is correct\n");
} else {
printf("Your password sucks\n");
}
return 0;
}
void
password_read(char password[]) {
printf("Enter password:\n");
scanf("%s", password);
}
int
password_check(char password[]) {
int upper = 0;
int digit = 0;
int dollar = 0;
int i;
for (i = 0; i < strlen(password); i++) {
if (isupper(password[i])) {
upper = 1;
}
else if (isdigit(password[i])) {
digit = 1;
}
else if (password[i] == '
I'm pretty shabby with C and I was just wondering if there was a way to reduce part of the code. I also just want to check if the pointer/array handling is correct, since I've only just started using it properly.) {
dollar = 1;
}
}
return (upper && digit && dollar);
}I'm pretty shabby with C and I was just wondering if there was a way to reduce part of the code. I also just want to check if the pointer/array handling is correct, since I've only just started using it properly.
Solution
Design
-
So the point of this program is to check a password, right? There are many problems with your approach to verifying a password. To list a few:
-
Password could be brute-forced in a matter of milliseconds (on a slow machine) due to small length
-
Anyone could reverse-engineer your code by de-compiling it and find out how to enter a valid password, even if length was increased to something not as easily brute-forced.
-
You aren't masking the password when it's entered into the command line in any way; someone looking over your shoulder now knows your password
-
Your
Readability & Maintainability
-
Understandable variable and function names, good
-
No documentation, despite it being a small and easy to understand program you should still include this
-
Should have a header for those function prototypes, and perhaps a separate source file for the password handling functions.
-
You don't use command line arguments, declare
-
You should be using
-
You don't have to return
C99 & C11 §5.1.2.2(3)
...reaching the
value of
Functionality
-
While the program is only supposed to verify a password, the functionality isn't great. Why? Well look at the conditions for our password to be "correct" (quite a large range of strings will fall into this range):
-
an uppercase letter
-
a digit
-
a
-
So
Security
-
All in all, you shouldn't even be handling passwords by yourself. You should be relying on external libraries to get and encrypt the password.
-
To get a password, you could use
-
To hash a password, I would definitely go with OpenSSL. Please, when it comes to encryption, don't try and roll your own, or just find something someone posted on the Internet. Go with something like OpenSSL that is verified, and trusted by millions every day.
-
So the point of this program is to check a password, right? There are many problems with your approach to verifying a password. To list a few:
-
Password could be brute-forced in a matter of milliseconds (on a slow machine) due to small length
-
Anyone could reverse-engineer your code by de-compiling it and find out how to enter a valid password, even if length was increased to something not as easily brute-forced.
-
You aren't masking the password when it's entered into the command line in any way; someone looking over your shoulder now knows your password
-
Your
MAXSIZE is 100 when our password could be a length of 3? Or larger? We should be dynamically allocating the input variable.Readability & Maintainability
-
Understandable variable and function names, good
-
No documentation, despite it being a small and easy to understand program you should still include this
-
Should have a header for those function prototypes, and perhaps a separate source file for the password handling functions.
-
You don't use command line arguments, declare
main() as such:int main(void)-
You should be using
puts() instead of printf() since you are not formatting strings (this also lets us exclude the \n)-
You don't have to return
0 at the end of main(). The C standard knows how frequently this is used, and lets us omit it.C99 & C11 §5.1.2.2(3)
...reaching the
} that terminates the main() function returns avalue of
0. Functionality
-
While the program is only supposed to verify a password, the functionality isn't great. Why? Well look at the conditions for our password to be "correct" (quite a large range of strings will fall into this range):
-
an uppercase letter
-
a digit
-
a
$ character.-
So
H8$ is a valid password. Okay... but what if our password isn't "correct"? You output "Your password sucks". But that's actually quite unlikely, my passwords could be MUCH stronger than yours. Reference this relevent XKCD:Security
-
All in all, you shouldn't even be handling passwords by yourself. You should be relying on external libraries to get and encrypt the password.
-
To get a password, you could use
getpass(), but it is deprecated. See this answer for an alternative implementation.-
To hash a password, I would definitely go with OpenSSL. Please, when it comes to encryption, don't try and roll your own, or just find something someone posted on the Internet. Go with something like OpenSSL that is verified, and trusted by millions every day.
Code Snippets
int main(void)Context
StackExchange Code Review Q#132858, answer score: 15
Revisions (0)
No revisions yet.