{"action":"created","comment":{"url":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/comments/30791754","id":30791754,"diff_hunk":"@@ -172,34 +169,93 @@ impl<R: Read> Archive<R> {\n                                           \"empty file name in tarball\"))\n             }\n             let is_directory = bytes[bytes.len() - 1] == b'/';\n+\n+            // Notes regarding bsdtar 2.8.3 / libarchive 2.8.3:\n+            // * Leading '/'s are trimmed. For example, `///test` is treated as\n+            //   `test`.\n+            // * If the filename contains '..', then the file is skipped when\n+            //   extracting the tarball.\n+            // * '//' within a filename is effectively skipped. An error is\n+            //   logged, but otherwise the effect is as if any two or more\n+            //   adjacent '/'s within the filename were consolidated into one\n+            //   '/'.\n+\n             let mut dst = into.to_path_buf();\n+            let mut contains_dot_dot = false;\n+            let mut seen_non_empty_part = false;\n             for part in bytes.split(|x| *x == b'/') {\n+                if part == b\"..\" {\n+                    contains_dot_dot = true;\n+                    break;\n+                }\n+                // If empty, skip this part.\n+                // This effectively results in trimming leading '/'s as well as\n+                // merging adjacent '/'s.\n+                if part.len() == 0 {\n+                    continue;\n+                }\n+                seen_non_empty_part = true;\n                 push(&mut dst, part);\n             }\n+            // If any part of the filename is '..', then skip over unpacking the\n+            // file to prevent directory traversal security issues.  See, e.g.:\n+            // CVE-2001-1267, CVE-2002-0399, CVE-2005-1918, CVE-2007-4131\n+            if contains_dot_dot {\n+                continue;\n+            }\n+            // Skip cases where only slashes were seen, because this is\n+            // effectively an empty filename.\n+            if !seen_non_empty_part {\n+                continue;\n+            }\n+\n             if is_directory {\n-                try!(fs::create_dir_all(&dst));\n+                if let Err(e) = fs::create_dir_all(&dst) {","path":"src/lib.rs","position":68,"original_position":67,"commit_id":"e9db7161b788ed7c7953cb78e1e302fd1107265e","original_commit_id":"ff1b63adb1f8ec95bd69e5b7516c347f1beb05fb","user":{"login":"cadencemarseille","id":5514909,"avatar_url":"https://avatars.githubusercontent.com/u/5514909?v=3","gravatar_id":"","url":"https://api.github.com/users/cadencemarseille","html_url":"https://github.com/cadencemarseille","followers_url":"https://api.github.com/users/cadencemarseille/followers","following_url":"https://api.github.com/users/cadencemarseille/following{/other_user}","gists_url":"https://api.github.com/users/cadencemarseille/gists{/gist_id}","starred_url":"https://api.github.com/users/cadencemarseille/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cadencemarseille/subscriptions","organizations_url":"https://api.github.com/users/cadencemarseille/orgs","repos_url":"https://api.github.com/users/cadencemarseille/repos","events_url":"https://api.github.com/users/cadencemarseille/events{/privacy}","received_events_url":"https://api.github.com/users/cadencemarseille/received_events","type":"User","site_admin":false},"body":"The extracting_malicious_tarball() test depends on the second `if let Err(e) ...`.  What I have done in my new patch is add a test to check the first `if let Err(e) ...`.","created_at":"2015-05-21T11:00:06Z","updated_at":"2015-05-21T11:00:06Z","html_url":"https://github.com/alexcrichton/tar-rs/pull/25#discussion_r30791754","pull_request_url":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/25","_links":{"self":{"href":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/comments/30791754"},"html":{"href":"https://github.com/alexcrichton/tar-rs/pull/25#discussion_r30791754"},"pull_request":{"href":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/25"}}},"pull_request":{"url":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/25","id":35683239,"html_url":"https://github.com/alexcrichton/tar-rs/pull/25","diff_url":"https://github.com/alexcrichton/tar-rs/pull/25.diff","patch_url":"https://github.com/alexcrichton/tar-rs/pull/25.patch","issue_url":"https://api.github.com/repos/alexcrichton/tar-rs/issues/25","number":25,"state":"open","locked":false,"title":"Fix directory traversal security issues with Archive::unpack().","user":{"login":"cadencemarseille","id":5514909,"avatar_url":"https://avatars.githubusercontent.com/u/5514909?v=3","gravatar_id":"","url":"https://api.github.com/users/cadencemarseille","html_url":"https://github.com/cadencemarseille","followers_url":"https://api.github.com/users/cadencemarseille/followers","following_url":"https://api.github.com/users/cadencemarseille/following{/other_user}","gists_url":"https://api.github.com/users/cadencemarseille/gists{/gist_id}","starred_url":"https://api.github.com/users/cadencemarseille/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cadencemarseille/subscriptions","organizations_url":"https://api.github.com/users/cadencemarseille/orgs","repos_url":"https://api.github.com/users/cadencemarseille/repos","events_url":"https://api.github.com/users/cadencemarseille/events{/privacy}","received_events_url":"https://api.github.com/users/cadencemarseille/received_events","type":"User","site_admin":false},"body":"Similar to directory traversal security issues that have affected GNU tar in the\r\npast, Archive::unpack() should not allow '..' to be used to overwrite arbitrary\r\nfiles.\r\n\r\nWhile I was in this area, I also made the following changes:\r\n- Remove `#[cfg(feature = \"nightly\")]` and provide a fallback for unix and\r\n  Rust stable that calls libc::chmod() instead.\r\n- Fix the issue that set_perms() was not calling APIs that persisted the permissions\r\n  changes. Permissions::set_readonly() does not modify the filesystem and presumably\r\n  PermissionsExt::set_mode() does not, either.\r\n- Ignore AlreadyExists errors from fs::create_dir_all().","created_at":"2015-05-18T23:23:16Z","updated_at":"2015-05-21T11:00:06Z","closed_at":null,"merged_at":null,"merge_commit_sha":"343b6007485f6cd89da2ba10de072b6a363713f6","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/25/commits","review_comments_url":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/25/comments","review_comment_url":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/comments{/number}","comments_url":"https://api.github.com/repos/alexcrichton/tar-rs/issues/25/comments","statuses_url":"https://api.github.com/repos/alexcrichton/tar-rs/statuses/e9db7161b788ed7c7953cb78e1e302fd1107265e","head":{"label":"cadencemarseille:unpack-fixes","ref":"unpack-fixes","sha":"e9db7161b788ed7c7953cb78e1e302fd1107265e","user":{"login":"cadencemarseille","id":5514909,"avatar_url":"https://avatars.githubusercontent.com/u/5514909?v=3","gravatar_id":"","url":"https://api.github.com/users/cadencemarseille","html_url":"https://github.com/cadencemarseille","followers_url":"https://api.github.com/users/cadencemarseille/followers","following_url":"https://api.github.com/users/cadencemarseille/following{/other_user}","gists_url":"https://api.github.com/users/cadencemarseille/gists{/gist_id}","starred_url":"https://api.github.com/users/cadencemarseille/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cadencemarseille/subscriptions","organizations_url":"https://api.github.com/users/cadencemarseille/orgs","repos_url":"https://api.github.com/users/cadencemarseille/repos","events_url":"https://api.github.com/users/cadencemarseille/events{/privacy}","received_events_url":"https://api.github.com/users/cadencemarseille/received_events","type":"User","site_admin":false},"repo":{"id":35847645,"name":"tar-rs","full_name":"cadencemarseille/tar-rs","owner":{"login":"cadencemarseille","id":5514909,"avatar_url":"https://avatars.githubusercontent.com/u/5514909?v=3","gravatar_id":"","url":"https://api.github.com/users/cadencemarseille","html_url":"https://github.com/cadencemarseille","followers_url":"https://api.github.com/users/cadencemarseille/followers","following_url":"https://api.github.com/users/cadencemarseille/following{/other_user}","gists_url":"https://api.github.com/users/cadencemarseille/gists{/gist_id}","starred_url":"https://api.github.com/users/cadencemarseille/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cadencemarseille/subscriptions","organizations_url":"https://api.github.com/users/cadencemarseille/orgs","repos_url":"https://api.github.com/users/cadencemarseille/repos","events_url":"https://api.github.com/users/cadencemarseille/events{/privacy}","received_events_url":"https://api.github.com/users/cadencemarseille/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/cadencemarseille/tar-rs","description":"Tar file reading/writing for Rust","fork":true,"url":"https://api.github.com/repos/cadencemarseille/tar-rs","forks_url":"https://api.github.com/repos/cadencemarseille/tar-rs/forks","keys_url":"https://api.github.com/repos/cadencemarseille/tar-rs/keys{/key_id}","collaborators_url":"https://api.github.com/repos/cadencemarseille/tar-rs/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/cadencemarseille/tar-rs/teams","hooks_url":"https://api.github.com/repos/cadencemarseille/tar-rs/hooks","issue_events_url":"https://api.github.com/repos/cadencemarseille/tar-rs/issues/events{/number}","events_url":"https://api.github.com/repos/cadencemarseille/tar-rs/events","assignees_url":"https://api.github.com/repos/cadencemarseille/tar-rs/assignees{/user}","branches_url":"https://api.github.com/repos/cadencemarseille/tar-rs/branches{/branch}","tags_url":"https://api.github.com/repos/cadencemarseille/tar-rs/tags","blobs_url":"https://api.github.com/repos/cadencemarseille/tar-rs/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/cadencemarseille/tar-rs/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/cadencemarseille/tar-rs/git/refs{/sha}","trees_url":"https://api.github.com/repos/cadencemarseille/tar-rs/git/trees{/sha}","statuses_url":"https://api.github.com/repos/cadencemarseille/tar-rs/statuses/{sha}","languages_url":"https://api.github.com/repos/cadencemarseille/tar-rs/languages","stargazers_url":"https://api.github.com/repos/cadencemarseille/tar-rs/stargazers","contributors_url":"https://api.github.com/repos/cadencemarseille/tar-rs/contributors","subscribers_url":"https://api.github.com/repos/cadencemarseille/tar-rs/subscribers","subscription_url":"https://api.github.com/repos/cadencemarseille/tar-rs/subscription","commits_url":"https://api.github.com/repos/cadencemarseille/tar-rs/commits{/sha}","git_commits_url":"https://api.github.com/repos/cadencemarseille/tar-rs/git/commits{/sha}","comments_url":"https://api.github.com/repos/cadencemarseille/tar-rs/comments{/number}","issue_comment_url":"https://api.github.com/repos/cadencemarseille/tar-rs/issues/comments{/number}","contents_url":"https://api.github.com/repos/cadencemarseille/tar-rs/contents/{+path}","compare_url":"https://api.github.com/repos/cadencemarseille/tar-rs/compare/{base}...{head}","merges_url":"https://api.github.com/repos/cadencemarseille/tar-rs/merges","archive_url":"https://api.github.com/repos/cadencemarseille/tar-rs/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/cadencemarseille/tar-rs/downloads","issues_url":"https://api.github.com/repos/cadencemarseille/tar-rs/issues{/number}","pulls_url":"https://api.github.com/repos/cadencemarseille/tar-rs/pulls{/number}","milestones_url":"https://api.github.com/repos/cadencemarseille/tar-rs/milestones{/number}","notifications_url":"https://api.github.com/repos/cadencemarseille/tar-rs/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/cadencemarseille/tar-rs/labels{/name}","releases_url":"https://api.github.com/repos/cadencemarseille/tar-rs/releases{/id}","created_at":"2015-05-18T23:20:46Z","updated_at":"2015-05-18T23:20:46Z","pushed_at":"2015-05-21T10:52:43Z","git_url":"git://github.com/cadencemarseille/tar-rs.git","ssh_url":"git@github.com:cadencemarseille/tar-rs.git","clone_url":"https://github.com/cadencemarseille/tar-rs.git","svn_url":"https://github.com/cadencemarseille/tar-rs","homepage":"http://alexcrichton.com/tar-rs","size":1026,"stargazers_count":0,"watchers_count":0,"language":"Rust","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"alexcrichton:master","ref":"master","sha":"288a9754a8205b1f032e1c92015e4dbe49998855","user":{"login":"alexcrichton","id":64996,"avatar_url":"https://avatars.githubusercontent.com/u/64996?v=3","gravatar_id":"","url":"https://api.github.com/users/alexcrichton","html_url":"https://github.com/alexcrichton","followers_url":"https://api.github.com/users/alexcrichton/followers","following_url":"https://api.github.com/users/alexcrichton/following{/other_user}","gists_url":"https://api.github.com/users/alexcrichton/gists{/gist_id}","starred_url":"https://api.github.com/users/alexcrichton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexcrichton/subscriptions","organizations_url":"https://api.github.com/users/alexcrichton/orgs","repos_url":"https://api.github.com/users/alexcrichton/repos","events_url":"https://api.github.com/users/alexcrichton/events{/privacy}","received_events_url":"https://api.github.com/users/alexcrichton/received_events","type":"User","site_admin":false},"repo":{"id":21946092,"name":"tar-rs","full_name":"alexcrichton/tar-rs","owner":{"login":"alexcrichton","id":64996,"avatar_url":"https://avatars.githubusercontent.com/u/64996?v=3","gravatar_id":"","url":"https://api.github.com/users/alexcrichton","html_url":"https://github.com/alexcrichton","followers_url":"https://api.github.com/users/alexcrichton/followers","following_url":"https://api.github.com/users/alexcrichton/following{/other_user}","gists_url":"https://api.github.com/users/alexcrichton/gists{/gist_id}","starred_url":"https://api.github.com/users/alexcrichton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexcrichton/subscriptions","organizations_url":"https://api.github.com/users/alexcrichton/orgs","repos_url":"https://api.github.com/users/alexcrichton/repos","events_url":"https://api.github.com/users/alexcrichton/events{/privacy}","received_events_url":"https://api.github.com/users/alexcrichton/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/alexcrichton/tar-rs","description":"Tar file reading/writing for Rust","fork":false,"url":"https://api.github.com/repos/alexcrichton/tar-rs","forks_url":"https://api.github.com/repos/alexcrichton/tar-rs/forks","keys_url":"https://api.github.com/repos/alexcrichton/tar-rs/keys{/key_id}","collaborators_url":"https://api.github.com/repos/alexcrichton/tar-rs/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/alexcrichton/tar-rs/teams","hooks_url":"https://api.github.com/repos/alexcrichton/tar-rs/hooks","issue_events_url":"https://api.github.com/repos/alexcrichton/tar-rs/issues/events{/number}","events_url":"https://api.github.com/repos/alexcrichton/tar-rs/events","assignees_url":"https://api.github.com/repos/alexcrichton/tar-rs/assignees{/user}","branches_url":"https://api.github.com/repos/alexcrichton/tar-rs/branches{/branch}","tags_url":"https://api.github.com/repos/alexcrichton/tar-rs/tags","blobs_url":"https://api.github.com/repos/alexcrichton/tar-rs/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/alexcrichton/tar-rs/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/alexcrichton/tar-rs/git/refs{/sha}","trees_url":"https://api.github.com/repos/alexcrichton/tar-rs/git/trees{/sha}","statuses_url":"https://api.github.com/repos/alexcrichton/tar-rs/statuses/{sha}","languages_url":"https://api.github.com/repos/alexcrichton/tar-rs/languages","stargazers_url":"https://api.github.com/repos/alexcrichton/tar-rs/stargazers","contributors_url":"https://api.github.com/repos/alexcrichton/tar-rs/contributors","subscribers_url":"https://api.github.com/repos/alexcrichton/tar-rs/subscribers","subscription_url":"https://api.github.com/repos/alexcrichton/tar-rs/subscription","commits_url":"https://api.github.com/repos/alexcrichton/tar-rs/commits{/sha}","git_commits_url":"https://api.github.com/repos/alexcrichton/tar-rs/git/commits{/sha}","comments_url":"https://api.github.com/repos/alexcrichton/tar-rs/comments{/number}","issue_comment_url":"https://api.github.com/repos/alexcrichton/tar-rs/issues/comments{/number}","contents_url":"https://api.github.com/repos/alexcrichton/tar-rs/contents/{+path}","compare_url":"https://api.github.com/repos/alexcrichton/tar-rs/compare/{base}...{head}","merges_url":"https://api.github.com/repos/alexcrichton/tar-rs/merges","archive_url":"https://api.github.com/repos/alexcrichton/tar-rs/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/alexcrichton/tar-rs/downloads","issues_url":"https://api.github.com/repos/alexcrichton/tar-rs/issues{/number}","pulls_url":"https://api.github.com/repos/alexcrichton/tar-rs/pulls{/number}","milestones_url":"https://api.github.com/repos/alexcrichton/tar-rs/milestones{/number}","notifications_url":"https://api.github.com/repos/alexcrichton/tar-rs/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/alexcrichton/tar-rs/labels{/name}","releases_url":"https://api.github.com/repos/alexcrichton/tar-rs/releases{/id}","created_at":"2014-07-17T14:56:29Z","updated_at":"2015-05-03T17:52:42Z","pushed_at":"2015-05-21T10:52:44Z","git_url":"git://github.com/alexcrichton/tar-rs.git","ssh_url":"git@github.com:alexcrichton/tar-rs.git","clone_url":"https://github.com/alexcrichton/tar-rs.git","svn_url":"https://github.com/alexcrichton/tar-rs","homepage":"http://alexcrichton.com/tar-rs","size":2496,"stargazers_count":11,"watchers_count":11,"language":"Rust","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":14,"mirror_url":null,"open_issues_count":3,"forks":14,"open_issues":3,"watchers":11,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/25"},"html":{"href":"https://github.com/alexcrichton/tar-rs/pull/25"},"issue":{"href":"https://api.github.com/repos/alexcrichton/tar-rs/issues/25"},"comments":{"href":"https://api.github.com/repos/alexcrichton/tar-rs/issues/25/comments"},"review_comments":{"href":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/25/comments"},"review_comment":{"href":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/alexcrichton/tar-rs/pulls/25/commits"},"statuses":{"href":"https://api.github.com/repos/alexcrichton/tar-rs/statuses/e9db7161b788ed7c7953cb78e1e302fd1107265e"}}}}