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

igetwild bash cleanup #3452

Closed
trel opened this issue Jan 20, 2017 · 3 comments
Closed

igetwild bash cleanup #3452

trel opened this issue Jan 20, 2017 · 3 comments
Assignees
Labels
Milestone

Comments

@trel
Copy link
Member

trel commented Jan 20, 2017

No description provided.

@trel trel added the bug label Jan 20, 2017
@trel trel added this to the 4.1.11 milestone Jan 20, 2017
@trel trel self-assigned this Jan 20, 2017
trel added a commit to trel/irods that referenced this issue Mar 17, 2017
trel added a commit that referenced this issue May 3, 2017
trel pushed a commit to irods/irods_client_icommands that referenced this issue May 3, 2017
Use bash -e

set -e is good practice because it means the script will exit if a
command fails, rather than carrying on regardless.

Quote variables everywhere in igetwild

It's good practice to always quote shell variables (consider, for
instance, what would happen if TMPDIR contained a space).

Remove tmpfiles on error in igetwild

Only remove the tmpfiles at the end of a successful
run, or when an error occurs.

Signed-off-by: Matthew Vernon <mv3@sanger.ac.uk>
Signed-off-by: Terrell Russell <terrellrussell@gmail.com>
trel pushed a commit to irods/irods_client_icommands that referenced this issue May 3, 2017
…in igetwild

We extract a list of filenames from iquest, and then pass
them to iget one by one using xargs.

The complication is that filenames can contain pretty much any
character, and iquest doesn't have the option to separate output
records with the NULL byte. So we use the closest we can, which is /\n
(iquest always adds the \n). We then replace that /\n sequence with
NULL. That gets us most of the way there, but our sequence will still
finish "/\n", because sed doesn't see the final newline in its pattern
space. So we replace the final / with NULL and then use head to remove
the final character (the \n).

Signed-off-by: Matthew Vernon <mv3@sanger.ac.uk>
Signed-off-by: Terrell Russell <terrellrussell@gmail.com>
trel added a commit to irods/irods_client_icommands that referenced this issue May 3, 2017
trel pushed a commit that referenced this issue May 3, 2017
Use bash -e

set -e is good practice because it means the script will exit if a
command fails, rather than carrying on regardless.

Quote variables everywhere in igetwild

It's good practice to always quote shell variables (consider, for
instance, what would happen if TMPDIR contained a space).

Remove tmpfiles on error in igetwild

Only remove the tmpfiles at the end of a successful
run, or when an error occurs.

Signed-off-by: Matthew Vernon <mv3@sanger.ac.uk>
Signed-off-by: Terrell Russell <terrellrussell@gmail.com>
trel pushed a commit that referenced this issue May 3, 2017
We extract a list of filenames from iquest, and then pass
them to iget one by one using xargs.

The complication is that filenames can contain pretty much any
character, and iquest doesn't have the option to separate output
records with the NULL byte. So we use the closest we can, which is /\n
(iquest always adds the \n). We then replace that /\n sequence with
NULL. That gets us most of the way there, but our sequence will still
finish "/\n", because sed doesn't see the final newline in its pattern
space. So we replace the final / with NULL and then use head to remove
the final character (the \n).

Signed-off-by: Matthew Vernon <mv3@sanger.ac.uk>
Signed-off-by: Terrell Russell <terrellrussell@gmail.com>
@trel
Copy link
Member Author

trel commented May 3, 2017

needs cherry-pick to 4-2-stable.

trel pushed a commit to irods/irods_client_icommands that referenced this issue May 3, 2017
Use bash -e

set -e is good practice because it means the script will exit if a
command fails, rather than carrying on regardless.

Quote variables everywhere in igetwild

It's good practice to always quote shell variables (consider, for
instance, what would happen if TMPDIR contained a space).

Remove tmpfiles on error in igetwild

Only remove the tmpfiles at the end of a successful
run, or when an error occurs.

Signed-off-by: Matthew Vernon <mv3@sanger.ac.uk>
Signed-off-by: Terrell Russell <terrellrussell@gmail.com>
trel pushed a commit to irods/irods_client_icommands that referenced this issue May 3, 2017
…in igetwild

We extract a list of filenames from iquest, and then pass
them to iget one by one using xargs.

The complication is that filenames can contain pretty much any
character, and iquest doesn't have the option to separate output
records with the NULL byte. So we use the closest we can, which is /\n
(iquest always adds the \n). We then replace that /\n sequence with
NULL. That gets us most of the way there, but our sequence will still
finish "/\n", because sed doesn't see the final newline in its pattern
space. So we replace the final / with NULL and then use head to remove
the final character (the \n).

Signed-off-by: Matthew Vernon <mv3@sanger.ac.uk>
Signed-off-by: Terrell Russell <terrellrussell@gmail.com>
trel added a commit to irods/irods_client_icommands that referenced this issue May 3, 2017
trel added a commit that referenced this issue May 3, 2017
@trel
Copy link
Member Author

trel commented May 3, 2017

thanks @mcv21

@trel trel closed this as completed May 3, 2017
@trel
Copy link
Member Author

trel commented May 5, 2017

See http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-8799

[Description]
Untrusted input execution via igetwild in all iRODS versions before
4.1.11 and 4.2.1 allows other iRODS users (potentially anonymous) to
execute remote shell commands via iRODS virtual pathnames.

[Attack Vectors]
To exploit this vulnerability, a virtual iRODS pathname which includes
a semicolon would be retrieved via igetwild. Because igetwild is a
bash script, the part of the pathname following the semicolon would be
executed in the user's shell.

xu-hao pushed a commit to xu-hao/temporary-irods-mod that referenced this issue Jun 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

1 participant