Skip to content

Commit 0640dbe

Browse files
authored
fix: remove invalid unpacking logic for IPFS pinned pkg sources (#6902)
## Description closes #6721. There were couple of issues: 1. Archive unpacking was getting the bytes from `text` field of response to the IPFS gateway which was adding additional stuff that messes with unpacking routine. 2. Folder name was invalid so forc was unable to find the packages in the `ipfs` cache. Both issues are addressed and this unblocks forc to fetch ipfs fetched packages once again. Next up on our package registry push is to enable alternative sources and set the precedence for them. Also added some tests around extracting logic, which inserts the contents under a directory named after `CID` as this is how `forc` is looking for these sources in the first place (related to point 2 above)
1 parent 1fdf630 commit 0640dbe

File tree

4 files changed

+172
-20
lines changed

4 files changed

+172
-20
lines changed

Cargo.lock

+3-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ etk-ops = { package = "fuel-etk-ops", version = "0.3.1-dev" }
140140
extension-trait = "1.0"
141141
fd-lock = "4.0"
142142
filecheck = "0.5"
143+
flate2 = "1.0"
143144
fs_extra = "1.2"
144145
futures = { version = "0.3", default-features = false }
145146
gag = "1.0"

forc-pkg/Cargo.toml

+2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ ansiterm.workspace = true
1313
anyhow.workspace = true
1414
byte-unit.workspace = true
1515
cid.workspace = true
16+
flate2.workspace = true
1617
forc-tracing.workspace = true
1718
forc-util.workspace = true
1819
fuel-abi-types.workspace = true
@@ -42,6 +43,7 @@ walkdir.workspace = true
4243

4344
[dev-dependencies]
4445
regex = "^1.10.2"
46+
tempfile.workspace = true
4547

4648
[target.'cfg(not(target_os = "macos"))'.dependencies]
4749
sysinfo = "0.29"

forc-pkg/src/source/ipfs.rs

+166-19
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::{
44
source,
55
};
66
use anyhow::Result;
7+
use flate2::read::GzDecoder;
78
use forc_tracing::println_action_green;
89
use futures::TryStreamExt;
910
use ipfs_api::IpfsApi;
@@ -130,6 +131,18 @@ impl fmt::Display for Pinned {
130131
}
131132

132133
impl Cid {
134+
fn extract_archive<R: std::io::Read>(&self, reader: R, dst: &Path) -> Result<()> {
135+
let dst_dir = dst.join(self.0.to_string());
136+
std::fs::create_dir_all(&dst_dir)?;
137+
let mut archive = Archive::new(reader);
138+
139+
for entry in archive.entries()? {
140+
let mut entry = entry?;
141+
entry.unpack_in(&dst_dir)?;
142+
}
143+
144+
Ok(())
145+
}
133146
/// Using local node, fetches the content described by this cid.
134147
async fn fetch_with_client(&self, ipfs_client: &IpfsClient, dst: &Path) -> Result<()> {
135148
let cid_path = format!("/ipfs/{}", self.0);
@@ -140,8 +153,7 @@ impl Cid {
140153
.try_concat()
141154
.await?;
142155
// After collecting bytes of the archive, we unpack it to the dst.
143-
let mut archive = Archive::new(bytes.as_slice());
144-
archive.unpack(dst)?;
156+
self.extract_archive(bytes.as_slice(), dst)?;
145157
Ok(())
146158
}
147159

@@ -150,19 +162,18 @@ impl Cid {
150162
let client = reqwest::Client::new();
151163
// We request the content to be served to us in tar format by the public gateway.
152164
let fetch_url = format!(
153-
"{}/ipfs/{}?download=true&format=tar&filename={}.tar",
165+
"{}/ipfs/{}?download=true&filename={}.tar.gz",
154166
gateway_url, self.0, self.0
155167
);
156168
let req = client.get(&fetch_url);
157169
let res = req.send().await?;
158170
if !res.status().is_success() {
159171
anyhow::bail!("Failed to fetch from {fetch_url:?}");
160172
}
161-
let bytes: Vec<_> = res.text().await?.bytes().collect();
162-
163-
// After collecting bytes of the archive, we unpack it to the dst.
164-
let mut archive = Archive::new(bytes.as_slice());
165-
archive.unpack(dst)?;
173+
let bytes: Vec<_> = res.bytes().await?.into_iter().collect();
174+
let tar = GzDecoder::new(bytes.as_slice());
175+
// After collecting and decoding bytes of the archive, we unpack it to the dst.
176+
self.extract_archive(tar, dst)?;
166177
Ok(())
167178
}
168179
}
@@ -225,18 +236,154 @@ fn pkg_cache_dir(cid: &Cid) -> PathBuf {
225236
fn ipfs_client() -> IpfsClient {
226237
IpfsClient::default()
227238
}
239+
#[cfg(test)]
240+
mod tests {
241+
use super::*;
242+
use anyhow::Result;
243+
use std::io::Cursor;
244+
use tar::Header;
245+
use tempfile::TempDir;
246+
247+
fn create_header(path: &str, size: u64) -> Header {
248+
let mut header = Header::new_gnu();
249+
header.set_path(path).unwrap();
250+
header.set_size(size);
251+
header.set_mode(0o755);
252+
header.set_cksum();
253+
header
254+
}
255+
256+
fn create_test_tar(files: &[(&str, &str)]) -> Vec<u8> {
257+
let mut ar = tar::Builder::new(Vec::new());
258+
259+
// Add root project directory
260+
let header = create_header("test-project/", 0);
261+
ar.append(&header, &mut std::io::empty()).unwrap();
262+
263+
// Add files
264+
for (path, content) in files {
265+
let full_path = format!("test-project/{}", path);
266+
let header = create_header(&full_path, content.len() as u64);
267+
ar.append(&header, content.as_bytes()).unwrap();
268+
}
269+
270+
ar.into_inner().unwrap()
271+
}
272+
273+
fn create_test_cid() -> Cid {
274+
let cid = cid::Cid::from_str("QmYwAPJzv5CZsnA625s3Xf2nemtYgPpHdWEz79ojWnPbdG").unwrap();
275+
Cid(cid)
276+
}
277+
278+
#[test]
279+
fn test_basic_extraction() -> Result<()> {
280+
let temp_dir = TempDir::new()?;
281+
let cid = create_test_cid();
228282

229-
#[test]
230-
fn test_source_ipfs_pinned_parsing() {
231-
let string = "ipfs+QmYwAPJzv5CZsnA625s3Xf2nemtYgPpHdWEz79ojWnPbdG";
283+
let tar_content = create_test_tar(&[("test.txt", "hello world")]);
284+
285+
cid.extract_archive(Cursor::new(tar_content), temp_dir.path())?;
286+
287+
let extracted_path = temp_dir
288+
.path()
289+
.join(cid.0.to_string())
290+
.join("test-project")
291+
.join("test.txt");
292+
293+
assert!(extracted_path.exists());
294+
assert_eq!(std::fs::read_to_string(extracted_path)?, "hello world");
295+
296+
Ok(())
297+
}
232298

233-
let expected = Pinned(Cid(cid::Cid::from_str(
234-
"QmYwAPJzv5CZsnA625s3Xf2nemtYgPpHdWEz79ojWnPbdG",
235-
)
236-
.unwrap()));
299+
#[test]
300+
fn test_nested_files() -> Result<()> {
301+
let temp_dir = TempDir::new()?;
302+
let cid = create_test_cid();
237303

238-
let parsed = Pinned::from_str(string).unwrap();
239-
assert_eq!(parsed, expected);
240-
let serialized = expected.to_string();
241-
assert_eq!(&serialized, string);
304+
let tar_content =
305+
create_test_tar(&[("src/main.sw", "contract {};"), ("README.md", "# Test")]);
306+
307+
cid.extract_archive(Cursor::new(tar_content), temp_dir.path())?;
308+
309+
let base = temp_dir.path().join(cid.0.to_string()).join("test-project");
310+
assert_eq!(
311+
std::fs::read_to_string(base.join("src/main.sw"))?,
312+
"contract {};"
313+
);
314+
assert_eq!(std::fs::read_to_string(base.join("README.md"))?, "# Test");
315+
316+
Ok(())
317+
}
318+
319+
#[test]
320+
fn test_invalid_tar() {
321+
let temp_dir = TempDir::new().unwrap();
322+
let cid = create_test_cid();
323+
324+
let result = cid.extract_archive(Cursor::new(b"not a tar file"), temp_dir.path());
325+
326+
assert!(result.is_err());
327+
}
328+
329+
#[test]
330+
fn test_source_ipfs_pinned_parsing() {
331+
let string = "ipfs+QmYwAPJzv5CZsnA625s3Xf2nemtYgPpHdWEz79ojWnPbdG";
332+
let expected = Pinned(Cid(cid::Cid::from_str(
333+
"QmYwAPJzv5CZsnA625s3Xf2nemtYgPpHdWEz79ojWnPbdG",
334+
)
335+
.unwrap()));
336+
let parsed = Pinned::from_str(string).unwrap();
337+
assert_eq!(parsed, expected);
338+
let serialized = expected.to_string();
339+
assert_eq!(&serialized, string);
340+
}
341+
342+
#[test]
343+
fn test_path_traversal_prevention() -> Result<()> {
344+
let temp_dir = TempDir::new()?;
345+
let cid = create_test_cid();
346+
347+
// Create a known directory structure
348+
let target_dir = temp_dir.path().join("target");
349+
std::fs::create_dir(&target_dir)?;
350+
351+
// Create our canary file in a known location
352+
let canary_content = "sensitive content";
353+
let canary_path = target_dir.join("canary.txt");
354+
std::fs::write(&canary_path, canary_content)?;
355+
356+
// Create tar with malicious path targeting our specific canary file
357+
let mut header = tar::Header::new_gnu();
358+
let malicious_path = b"../../target/canary.txt";
359+
header.as_gnu_mut().unwrap().name[..malicious_path.len()].copy_from_slice(malicious_path);
360+
header.set_size(17);
361+
header.set_mode(0o644);
362+
header.set_cksum();
363+
364+
let mut ar = tar::Builder::new(Vec::new());
365+
ar.append(&header, b"malicious content".as_slice())?;
366+
367+
// Add safe file
368+
let mut safe_header = tar::Header::new_gnu();
369+
safe_header.set_path("safe.txt")?;
370+
safe_header.set_size(12);
371+
safe_header.set_mode(0o644);
372+
safe_header.set_cksum();
373+
ar.append(&safe_header, b"safe content".as_slice())?;
374+
375+
// Extract to a subdirectory of temp_dir
376+
let tar_content = ar.into_inner()?;
377+
let extract_dir = temp_dir.path().join("extract");
378+
std::fs::create_dir(&extract_dir)?;
379+
cid.extract_archive(Cursor::new(tar_content), &extract_dir)?;
380+
381+
// Verify canary file was not modified
382+
assert_eq!(
383+
std::fs::read_to_string(&canary_path)?,
384+
canary_content,
385+
"Canary file was modified - path traversal protection failed!"
386+
);
387+
Ok(())
388+
}
242389
}

0 commit comments

Comments
 (0)