Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osc add of directories does not quote the argument #340

Closed
Vogtinator opened this issue Sep 27, 2017 · 8 comments
Closed

osc add of directories does not quote the argument #340

Vogtinator opened this issue Sep 27, 2017 · 8 comments

Comments

@Vogtinator
Copy link
Member

This makes it impossible to add directories with certain characters in the name and allows "fun" like this:

> mkdir ';sh #'
> osc add ';sh #'
;sh # is a directory, do you want to archive it for submission? (y/n) y
.
./;sh #
./.osc
./.osc/_meta
./.osc/_files
./.osc/_osclib_version
./.osc/_apiurl
./.osc/_package
./.osc/_project
sh-4.4$ 
@marcus-h
Copy link
Member

marcus-h commented Sep 27, 2017 via email

@Vogtinator
Copy link
Member Author

Vogtinator commented Sep 28, 2017

That commit is incomplete and does not protect at all. Injection with ' is still an issue:

> mkdir "'"';sh #'
> osc add "'"';sh #'
';sh # is a directory, do you want to archive it for submission? (y/n) y
find: '': No such file or directory
sh-4.4$

marcus-h added a commit that referenced this issue Sep 28, 2017
This fixes a potential shell injection.

See also: #340 ("osc add of directories does not quote the argument")
marcus-h added a commit that referenced this issue Sep 28, 2017
This is a follow-up commit for commit c9c0f8a. Using core.run_external
with shell=True is too error-prone.

Fixes: #340 ("osc add of directories does not quote the argument")
@marcus-h
Copy link
Member

marcus-h commented Sep 28, 2017 via email

@Vogtinator
Copy link
Member Author

This is still not quite complete, now filenames that represent valid options to the command, like "--help":

> mkdir ./--help
> LANG=C osc add ./--help
--help is a directory, do you want to archive it for submission? (y/n) y
cpio: Usage: find [-H] [-L] [-P] [-Olevel] [-D help|tree|search|stat|rates|opt|exec] [path...] [expression]: Cannot stat: No such file or directory
cpio: blank line ignored
[...]

@marcus-h marcus-h reopened this Sep 29, 2017
@marcus-h
Copy link
Member

marcus-h commented Oct 10, 2017 via email

@Vogtinator
Copy link
Member Author

Thanks again! Finally (this time really;) ) fixed in commit c3ba1fb.

Almost :P (Yes, extremely nitpicky now, but it is fun ;-) ):

> mkdir "$(echo -ne "dir\nname")"
> (cd dir*; touch file{1,2})
> osc add dir*
dir
name is a directory, do you want to archive it for submission? (y/n) y
cpio: dir: Cannot stat: No such file or directory
cpio: name/file1: Cannot stat: No such file or directory
cpio: dir: Cannot stat: No such file or directory
cpio: name/file2: Cannot stat: No such file or directory
1 block

The only characters not allowed in filenames are '\0' and '/', as / is a valid path only a 0 is a valid option, so I guess -0 should work?

@marcus-h
Copy link
Member

marcus-h commented Oct 11, 2017 via email

@Vogtinator
Copy link
Member Author

Hmm did you only check out commit c3ba1fb? This was already
fixed in commit a5c7611.

Ah, ok! I applied the diff that was linked here instead of cloning everything.
Now I really can't find anything wrong with the code anymore 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants