5

I am using this code to extract a password protected RAR file. I am using the std::system() function to invoke the RAR command. If I use the password in the std::system() function, it works. But as tries to pass the password as parameter, it doesn't. For example, if in this code if I use password pwd, it gives this error:

"pwd is not recognised as internal or external command, operable program or batch file."

But if I change the code and make it to system("rar e -ppwd wingen.rar"), it works.

Can anybody explain what mistake I am making? Thanks in advance!

Here's my code:

#include<stdio.h>
#include<stdlib.h>
int main(int argc, char **argv)
{
    char pword[20];
    printf("enter the pword : ");
    gets(pword);
    system(("rar e -p%s wingen.rar",pword));
    getchar();
    return 0;
}
Azeem
  • 11,148
  • 4
  • 27
  • 40
narayanpatra
  • 5,627
  • 13
  • 51
  • 60
  • 3
    You shouldn't use `gets()`. The manpage explicitly says `Never use gets(). [...] Use fgets() instead`. If `gets()` reads more characters than the size of your buffer, it will continue to store them past its end, which is dangerous. – Bertrand Marron Aug 22 '10 at 05:34
  • Also, read the [FAQ](http://stackoverflow.com/faq "This is a link to the FAQ"), especially the part that says: "When you have decided which answer is the most helpful to you, **mark it as the accepted answer by clicking on the check box outline to the left of the answer**. This lets other people know that you have received a good answer to your question." – Bertrand Marron Aug 22 '10 at 12:26

3 Answers3

18

system() only takes one argument - a const char*. In fact,

system( "rar e -p%s wingen.rar", pword );

won't compile - the compiler will complain that you've passed too many arguments to system(). The reason that:

system( "rar e -p%s wingen.rar", pword );

compiles is that you have wrapped your two strings in parenthesis. This has the effect of evaluating the expression inside, which consists of the comma operator operating on two strings. The comma operator has the effect of returning the value of the second argument, so you end up calling:

system( pword );

Which in your example is equivalent to:

system( "pwd" );

And pwd isn't a command on your system (although on POSIX systems it is... but I digress). What you want to do has been explained in the other answers but for completeness I'll mention it too - you need to format your string using sprintf:

char buff[256];
sprintf( buff, "rar e -p%s wingen.rar", pword );

or you can concatenate strings, which might be a little bit faster (although for such a short string, it probably won't make a difference):

char buff[256] = "rar e -p";
strcat( buff, pword );
strcat( buff, " wingen.rar" );
Azeem
  • 11,148
  • 4
  • 27
  • 40
Niki Yoshiuchi
  • 16,883
  • 1
  • 35
  • 44
  • 3
    +1 for correctly diagnosing why there was no compilation error. – Jonathan Leffler Aug 22 '10 at 05:30
  • This is an extremely patient and comprehensive answer. +1 – Tim Post Aug 22 '10 at 06:04
  • 1
    I missed that in my explaination, when compiling with -Wall gcc gives "left-hand operand of comma expression has no effect" – Sean A.O. Harney Aug 22 '10 at 11:19
  • @RBerteig yep, even if he uses snprintf like with my answer, or strcat like this, there is still an injection attack. "rar e -pXX ; rm -rf * ; wingen.rar" would be yielded from pwd "XX ; rm -rf * ; " – Sean A.O. Harney Aug 22 '10 at 11:28
  • @Sean, too true. Anyone who wants to know more about what we're on about here, should see http://bobby-tables.com/ for a classic example illustrated by xkcd. – RBerteig Aug 23 '10 at 01:27
6

The system() function receives a string as the argument.

It's prototype is:

int system(const char *command);

Build the string before passing it. Use snprintf() perhaps.

char buf[512];
snprintf(buf, sizeof(buf), "rar e -p%s wingen.rar", pword);
system(buf);

EDIT:
All of these solutions are bad ideas since there is an injection vulnerability with using system with un-sanitized input.

Even if he uses snprintf like with my answer, or strcat like the other, there is still a problem since system() (at least with /bin/sh on *nix systems) can execute multiple commands with a single function call.

system("rar e -pXX wingen.rar ; rm -rf * ; # wingen.rar")

would be yielded from:

pwd = "XX wingen.rar ; rm -rf * ; #"
Azeem
  • 11,148
  • 4
  • 27
  • 40
Sean A.O. Harney
  • 23,901
  • 4
  • 30
  • 30
5

The system() function doesn't work like printf(). You need to create the full string, and then call system():

char command[100];
sprintf(command, "rar e -p%s wingen.rar", pword);
system(command);

The code you have right now is using the comma operator, which results in your 'format string' being completely ignored by your program. What you have there is 100% equivalent to writing:

system(pword);

Which is presumably not what you wanted.

Carl Norum
  • 219,201
  • 40
  • 422
  • 469