Take the 2-minute tour ×
Server Fault is a question and answer site for professional system and network administrators. It's 100% free, no registration required.

I need to get a variable value by reading from user uploaded text file.

I am doing from a system's script:

resourceVersion=`cat userFile.txt`
mkdir $resourceVersion
...

Can the content of this file harm the system in any way when I do later use of the $resourceVersion variable, as argument to other commands?

share|improve this question
1  
What if the file contains foo ; rm -rf /*? –  David Schwartz Jul 22 '12 at 22:22
    
@DavidSchwartz it would tell you that no directory named "foo ; rm -rf /" exists in order to create * in. (try it (or at least something similar)) –  stew Jul 23 '12 at 3:07

2 Answers 2

up vote 8 down vote accepted

Rule 1 of writing secure code: You must sanitize your user input.

At the very least pass -- into you mkdir clause to prevent manipulating switches.

Currently it could be used to create directories in arbitrary locations with arbitrary permissions.

In of itself, it probably wont lead to a breach but you could pass stuff like:

-m 0777 /var/lib/mysql/newdb

To create a a new mysql database anyone could write into.

Theres also a denial of service attack to be had in here because each space is treated as a new directory.so you could pass 32760 new paths to be processed.

Finally, some pseudo filesystems are sensitive to new directories in ways you might not expect. On fedora for example directories like /sys/fs/cgroup/newcgroup could create a new cgroup. Also in the LIO subsystem writing directories into /sys/kernel/config/target could be used to export a block device over an iscsi network (then the whole contents of your device could be copied).

So, no dont do this.

share|improve this answer
    
It is great advice to sanitize your input. however, some of the things that you say might go wrong don't actually go wrong. There isn't a denial of service attack here. if the file contains "a b c", his commands create one directory with spaces in the name, not 3 directories. Similarly, "-m 0777 /var/lib/mysql/newdb" will be treated as a single directory name, not as 3 different arguments. He's only passing a single argument. You should easily be able to verify this stuff for yourself. –  stew Jul 23 '12 at 3:17
    
@stew you would be right if his code was mkdir "$resourceVersion" - but it isn't. –  faker Jul 23 '12 at 8:45
    
did you test? I did. –  stew Jul 23 '12 at 11:46
    
@stew yes I did. With both bash and sh. Both behaved as this answer suggests. –  faker Jul 24 '12 at 11:45

At the moment I can add a world-writable folder if userFile.txt contains -m 777 /.secretstorage
In my opinion that is already bad enough to not use such a construct.

It is correct that with the current script you posted it is not possible to inject a different command.
But depending on which other commands you will use this variable with it could get a lot worse.
For example, if you plan on cleaning up at the end of your program using this variable and rm...

share|improve this answer

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.