From 2ad8bf77c48dca16f1a82e7311ec9b1c95386c7b Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 6 Dec 2022 21:01:29 +0100 Subject: [PATCH 1/2] fix(identify): ascii-only versions --- core/commands/id.go | 6 +++--- core/commands/stat_dht.go | 9 +++++++-- core/commands/swarm.go | 3 ++- test/sharness/t0026-id.sh | 25 ++++++++++++++----------- version.go | 21 +++++++++++++++++++-- 5 files changed, 45 insertions(+), 19 deletions(-) diff --git a/core/commands/id.go b/core/commands/id.go index 887f93efe31..f8d6cbb1b4a 100644 --- a/core/commands/id.go +++ b/core/commands/id.go @@ -176,18 +176,18 @@ func printPeer(keyEnc ke.KeyEncoder, ps pstore.Peerstore, p peer.ID) (interface{ protocols, _ := ps.GetProtocols(p) // don't care about errors here. for _, p := range protocols { - info.Protocols = append(info.Protocols, string(p)) + info.Protocols = append(info.Protocols, version.TrimVersion(string(p))) } sort.Strings(info.Protocols) if v, err := ps.Get(p, "ProtocolVersion"); err == nil { if vs, ok := v.(string); ok { - info.ProtocolVersion = vs + info.ProtocolVersion = version.TrimVersion(vs) } } if v, err := ps.Get(p, "AgentVersion"); err == nil { if vs, ok := v.(string); ok { - info.AgentVersion = vs + info.AgentVersion = version.TrimVersion(vs) } } diff --git a/core/commands/stat_dht.go b/core/commands/stat_dht.go index e6006e439d3..54a2a847797 100644 --- a/core/commands/stat_dht.go +++ b/core/commands/stat_dht.go @@ -6,6 +6,7 @@ import ( "text/tabwriter" "time" + version "github.com/ipfs/kubo" cmdenv "github.com/ipfs/kubo/core/commands/cmdenv" cmds "github.com/ipfs/go-ipfs-cmds" @@ -92,7 +93,9 @@ This interface is not stable and may change from release to release. info := dhtPeerInfo{ID: p.String()} if ver, err := nd.Peerstore.Get(p, "AgentVersion"); err == nil { - info.AgentVersion, _ = ver.(string) + if vs, ok := ver.(string); ok { + info.AgentVersion = version.TrimVersion(vs) + } } else if err == pstore.ErrNotFound { // ignore } else { @@ -143,7 +146,9 @@ This interface is not stable and may change from release to release. info := dhtPeerInfo{ID: pi.Id.String()} if ver, err := nd.Peerstore.Get(pi.Id, "AgentVersion"); err == nil { - info.AgentVersion, _ = ver.(string) + if vs, ok := ver.(string); ok { + info.AgentVersion = version.TrimVersion(vs) + } } else if err == pstore.ErrNotFound { // ignore } else { diff --git a/core/commands/swarm.go b/core/commands/swarm.go index 1508efcb8a8..9e6d53c6712 100644 --- a/core/commands/swarm.go +++ b/core/commands/swarm.go @@ -13,6 +13,7 @@ import ( "time" files "github.com/ipfs/go-ipfs-files" + version "github.com/ipfs/kubo" "github.com/ipfs/kubo/commands" "github.com/ipfs/kubo/config" "github.com/ipfs/kubo/core/commands/cmdenv" @@ -284,7 +285,7 @@ var swarmPeersCmd = &cmds.Command{ } for _, s := range strs { - ci.Streams = append(ci.Streams, streamInfo{Protocol: string(s)}) + ci.Streams = append(ci.Streams, streamInfo{Protocol: version.TrimVersion(string(s))}) } } sort.Sort(&ci) diff --git a/test/sharness/t0026-id.sh b/test/sharness/t0026-id.sh index 5d6d3db094a..aeba9321d84 100755 --- a/test/sharness/t0026-id.sh +++ b/test/sharness/t0026-id.sh @@ -23,12 +23,13 @@ test_id_compute_agent() { fi AGENT_VERSION="$AGENT_VERSION$AGENT_SUFFIX" fi - echo "$AGENT_VERSION" + # limit of 64 characters: https://github.com/ipfs/kubo/pull/9465 + echo -n "$AGENT_VERSION" | head -c 64 } test_expect_success "checking AgentVersion" ' test_id_compute_agent > expected-agent-version && - ipfs id -f "\n" > actual-agent-version && + ipfs id -f "" > actual-agent-version && test_cmp expected-agent-version actual-agent-version ' @@ -52,20 +53,22 @@ test_expect_success "checking and converting ID of a random peer while offline" ' # agent-version-suffix (local, offline) -test_launch_ipfs_daemon --agent-version-suffix=test-suffix -test_expect_success "checking AgentVersion with suffix (local)" ' - test_id_compute_agent test-suffix > expected-agent-version && - ipfs id -f "\n" > actual-agent-version && - test_cmp expected-agent-version actual-agent-version +test_launch_ipfs_daemon --agent-version-suffix=test-suffix-ąę-123456789012345678901234567890 +test_expect_success "checking AgentVersion with suffix (local, only ascii chars)" ' + test_id_compute_agent "test-suffix--123456789012345678901234567890" > expected && + ipfs id -f "" > actual && + test_should_not_contain "ą" actual && + test_cmp expected actual ' # agent-version-suffix (over libp2p identify protocol) iptb testbed create -type localipfs -count 2 -init -startup_cluster 2 --agent-version-suffix=test-suffix-identify +startup_cluster 2 --agent-version-suffix=test-suffix-identify-óź-123456789012345678901234567890 test_expect_success "checking AgentVersion with suffix (fetched via libp2p identify protocol)" ' - ipfsi 0 id -f "\n" > expected-identify-agent-version && - ipfsi 1 id "$(ipfsi 0 config Identity.PeerID)" -f "\n" > actual-libp2p-identify-agent-version && - test_cmp expected-identify-agent-version actual-libp2p-identify-agent-version + test_id_compute_agent "test-suffix-identify--123456789012345678901234567890" > expected && + ipfsi 1 id "$(ipfsi 0 config Identity.PeerID)" -f "" > actual && + test_should_not_contain "ó" actual && + test_cmp expected actual ' test_kill_ipfs_daemon diff --git a/version.go b/version.go index 2762442eeab..26c5a7d04ac 100644 --- a/version.go +++ b/version.go @@ -2,6 +2,7 @@ package ipfs import ( "fmt" + "regexp" "runtime" "github.com/ipfs/kubo/repo/fsrepo" @@ -15,6 +16,8 @@ const CurrentVersionNumber = "0.18.0-dev" const ApiVersion = "/kubo/" + CurrentVersionNumber + "/" //nolint +const maxVersionLen = 64 + // GetUserAgentVersion is the libp2p user agent used by go-ipfs. // // Note: This will end in `/` when no commit is available. This is expected. @@ -26,13 +29,27 @@ func GetUserAgentVersion() string { } userAgent += userAgentSuffix } - return userAgent + return TrimVersion(userAgent) } var userAgentSuffix string +var onlyASCII = regexp.MustCompile("[[:^ascii:]]") func SetUserAgentSuffix(suffix string) { - userAgentSuffix = suffix + userAgentSuffix = TrimVersion(suffix) +} + +func TrimVersion(version string) string { + ascii := onlyASCII.ReplaceAllLiteralString(version, "") + chars := 0 + for i := range ascii { + if chars >= maxVersionLen { + ascii = ascii[:i] + break + } + chars++ + } + return ascii } type VersionInfo struct { From 3807757882a4d51fc7c29bb614303f7fa4e41737 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Fri, 12 Sep 2025 19:24:30 +0200 Subject: [PATCH 2/2] fix: sanitize libp2p identify strings per RFC 9839 replace control/format/surrogate chars with U+FFFD instead of deleting as deletion is a security risk preserve private use characters as specified enforce 128 rune limit on untrusted peer data https://www.rfc-editor.org/rfc/rfc9839.html --- core/commands/cmdutils/sanitize.go | 50 ++++++ core/commands/id.go | 5 +- core/commands/stat_dht.go | 6 +- core/commands/swarm.go | 8 +- docs/examples/kubo-as-a-library/go.mod | 1 + docs/examples/kubo-as-a-library/go.sum | 2 + test/cli/agent_version_unicode_test.go | 220 +++++++++++++++++++++++++ test/dependencies/go.mod | 1 + test/dependencies/go.sum | 2 + test/sharness/t0026-id.sh | 27 ++- version.go | 32 +--- 11 files changed, 302 insertions(+), 52 deletions(-) create mode 100644 core/commands/cmdutils/sanitize.go create mode 100644 test/cli/agent_version_unicode_test.go diff --git a/core/commands/cmdutils/sanitize.go b/core/commands/cmdutils/sanitize.go new file mode 100644 index 00000000000..4cd3d3f5908 --- /dev/null +++ b/core/commands/cmdutils/sanitize.go @@ -0,0 +1,50 @@ +package cmdutils + +import ( + "strings" + "unicode" +) + +const maxRunes = 128 + +// CleanAndTrim sanitizes untrusted strings from remote peers to prevent display issues +// across web UIs, terminals, and logs. It replaces control characters, format characters, +// and surrogates with U+FFFD (�), then enforces a maximum length of 128 runes. +// +// This follows the libp2p identify specification and RFC 9839 guidance: +// replacing problematic code points is preferred over deletion as deletion +// is a known security risk. +func CleanAndTrim(str string) string { + // Build sanitized result + var result []rune + for _, r := range str { + // Replace control characters (Cc) with U+FFFD - prevents terminal escapes, CR, LF, etc. + if unicode.Is(unicode.Cc, r) { + result = append(result, '\uFFFD') + continue + } + // Replace format characters (Cf) with U+FFFD - prevents RTL/LTR overrides, zero-width chars + if unicode.Is(unicode.Cf, r) { + result = append(result, '\uFFFD') + continue + } + // Replace surrogate characters (Cs) with U+FFFD - invalid in UTF-8 + if unicode.Is(unicode.Cs, r) { + result = append(result, '\uFFFD') + continue + } + // Private use characters (Co) are preserved per spec + result = append(result, r) + } + + // Convert to string and trim whitespace + sanitized := strings.TrimSpace(string(result)) + + // Enforce maximum length (128 runes, not bytes) + runes := []rune(sanitized) + if len(runes) > maxRunes { + return string(runes[:maxRunes]) + } + + return sanitized +} diff --git a/core/commands/id.go b/core/commands/id.go index 00b2a372216..58886699be3 100644 --- a/core/commands/id.go +++ b/core/commands/id.go @@ -12,6 +12,7 @@ import ( version "github.com/ipfs/kubo" "github.com/ipfs/kubo/core" "github.com/ipfs/kubo/core/commands/cmdenv" + "github.com/ipfs/kubo/core/commands/cmdutils" cmds "github.com/ipfs/go-ipfs-cmds" ke "github.com/ipfs/kubo/core/commands/keyencode" @@ -174,13 +175,13 @@ func printPeer(keyEnc ke.KeyEncoder, ps pstore.Peerstore, p peer.ID) (interface{ protocols, _ := ps.GetProtocols(p) // don't care about errors here. for _, proto := range protocols { - info.Protocols = append(info.Protocols, protocol.ID(version.TrimVersion(string(proto)))) + info.Protocols = append(info.Protocols, protocol.ID(cmdutils.CleanAndTrim(string(proto)))) } slices.Sort(info.Protocols) if v, err := ps.Get(p, "AgentVersion"); err == nil { if vs, ok := v.(string); ok { - info.AgentVersion = version.TrimVersion(vs) + info.AgentVersion = cmdutils.CleanAndTrim(vs) } } diff --git a/core/commands/stat_dht.go b/core/commands/stat_dht.go index 54a2a847797..b4345f570b5 100644 --- a/core/commands/stat_dht.go +++ b/core/commands/stat_dht.go @@ -6,8 +6,8 @@ import ( "text/tabwriter" "time" - version "github.com/ipfs/kubo" cmdenv "github.com/ipfs/kubo/core/commands/cmdenv" + "github.com/ipfs/kubo/core/commands/cmdutils" cmds "github.com/ipfs/go-ipfs-cmds" dht "github.com/libp2p/go-libp2p-kad-dht" @@ -94,7 +94,7 @@ This interface is not stable and may change from release to release. if ver, err := nd.Peerstore.Get(p, "AgentVersion"); err == nil { if vs, ok := ver.(string); ok { - info.AgentVersion = version.TrimVersion(vs) + info.AgentVersion = cmdutils.CleanAndTrim(vs) } } else if err == pstore.ErrNotFound { // ignore @@ -147,7 +147,7 @@ This interface is not stable and may change from release to release. if ver, err := nd.Peerstore.Get(pi.Id, "AgentVersion"); err == nil { if vs, ok := ver.(string); ok { - info.AgentVersion = version.TrimVersion(vs) + info.AgentVersion = cmdutils.CleanAndTrim(vs) } } else if err == pstore.ErrNotFound { // ignore diff --git a/core/commands/swarm.go b/core/commands/swarm.go index 67f8f3006ac..533ccc07814 100644 --- a/core/commands/swarm.go +++ b/core/commands/swarm.go @@ -15,10 +15,10 @@ import ( "text/tabwriter" "time" - version "github.com/ipfs/kubo" "github.com/ipfs/kubo/commands" "github.com/ipfs/kubo/config" "github.com/ipfs/kubo/core/commands/cmdenv" + "github.com/ipfs/kubo/core/commands/cmdutils" "github.com/ipfs/kubo/core/node/libp2p" "github.com/ipfs/kubo/repo" "github.com/ipfs/kubo/repo/fsrepo" @@ -292,7 +292,7 @@ var swarmPeersCmd = &cmds.Command{ } for _, s := range strs { - ci.Streams = append(ci.Streams, streamInfo{Protocol: version.TrimVersion(string(s))}) + ci.Streams = append(ci.Streams, streamInfo{Protocol: cmdutils.CleanAndTrim(string(s))}) } } @@ -479,14 +479,14 @@ func (ci *connInfo) identifyPeer(ps pstore.Peerstore, p peer.ID) (IdOutput, erro if protocols, err := ps.GetProtocols(p); err == nil { for _, proto := range protocols { - info.Protocols = append(info.Protocols, protocol.ID(version.TrimVersion(string(proto)))) + info.Protocols = append(info.Protocols, protocol.ID(cmdutils.CleanAndTrim(string(proto)))) } slices.Sort(info.Protocols) } if v, err := ps.Get(p, "AgentVersion"); err == nil { if vs, ok := v.(string); ok { - info.AgentVersion = version.TrimVersion(vs) + info.AgentVersion = cmdutils.CleanAndTrim(vs) } } diff --git a/docs/examples/kubo-as-a-library/go.mod b/docs/examples/kubo-as-a-library/go.mod index 5c675b68d7b..d61d1636c21 100644 --- a/docs/examples/kubo-as-a-library/go.mod +++ b/docs/examples/kubo-as-a-library/go.mod @@ -82,6 +82,7 @@ require ( github.com/ipfs/go-ds-measure v0.2.2 // indirect github.com/ipfs/go-ds-pebble v0.5.1 // indirect github.com/ipfs/go-fs-lock v0.1.1 // indirect + github.com/ipfs/go-ipfs-cmds v0.15.0 // indirect github.com/ipfs/go-ipfs-ds-help v1.1.1 // indirect github.com/ipfs/go-ipfs-pq v0.0.3 // indirect github.com/ipfs/go-ipfs-redirects-file v0.1.2 // indirect diff --git a/docs/examples/kubo-as-a-library/go.sum b/docs/examples/kubo-as-a-library/go.sum index 2db25adf8fb..0df24b8fd07 100644 --- a/docs/examples/kubo-as-a-library/go.sum +++ b/docs/examples/kubo-as-a-library/go.sum @@ -323,6 +323,8 @@ github.com/ipfs/go-fs-lock v0.1.1 h1:TecsP/Uc7WqYYatasreZQiP9EGRy4ZnKoG4yXxR33nw github.com/ipfs/go-fs-lock v0.1.1/go.mod h1:2goSXMCw7QfscHmSe09oXiR34DQeUdm+ei+dhonqly0= github.com/ipfs/go-ipfs-blockstore v1.3.1 h1:cEI9ci7V0sRNivqaOr0elDsamxXFxJMMMy7PTTDQNsQ= github.com/ipfs/go-ipfs-blockstore v1.3.1/go.mod h1:KgtZyc9fq+P2xJUiCAzbRdhhqJHvsw8u2Dlqy2MyRTE= +github.com/ipfs/go-ipfs-cmds v0.15.0 h1:nQDgKadrzyiFyYoZMARMIoVoSwe3gGTAfGvrWLeAQbQ= +github.com/ipfs/go-ipfs-cmds v0.15.0/go.mod h1:VABf/mv/wqvYX6hLG6Z+40eNAEw3FQO0bSm370Or3Wk= github.com/ipfs/go-ipfs-delay v0.0.0-20181109222059-70721b86a9a8/go.mod h1:8SP1YXK1M1kXuc4KJZINY3TQQ03J2rwBG9QfXmbRPrw= github.com/ipfs/go-ipfs-delay v0.0.1 h1:r/UXYyRcddO6thwOnhiznIAiSvxMECGgtv35Xs1IeRQ= github.com/ipfs/go-ipfs-delay v0.0.1/go.mod h1:8SP1YXK1M1kXuc4KJZINY3TQQ03J2rwBG9QfXmbRPrw= diff --git a/test/cli/agent_version_unicode_test.go b/test/cli/agent_version_unicode_test.go new file mode 100644 index 00000000000..732f13466e4 --- /dev/null +++ b/test/cli/agent_version_unicode_test.go @@ -0,0 +1,220 @@ +package cli + +import ( + "strings" + "testing" + + "github.com/ipfs/kubo/core/commands/cmdutils" + "github.com/stretchr/testify/assert" +) + +func TestCleanAndTrimUnicode(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "Basic ASCII", + input: "kubo/1.0.0", + expected: "kubo/1.0.0", + }, + { + name: "Polish characters preserved", + input: "test-ąęćłńóśźż", + expected: "test-ąęćłńóśźż", + }, + { + name: "Chinese characters preserved", + input: "版本-中文测试", + expected: "版本-中文测试", + }, + { + name: "Arabic text preserved", + input: "اختبار-العربية", + expected: "اختبار-العربية", + }, + { + name: "Emojis preserved", + input: "version-1.0-🚀-🎉", + expected: "version-1.0-🚀-🎉", + }, + { + name: "Complex Unicode with combining marks preserved", + input: "h̸̢̢̢̢̢̢̢̢̢̢e̵̵̵̵̵̵̵̵̵̵l̷̷̷̷̷̷̷̷̷̷l̶̶̶̶̶̶̶̶̶̶o̴̴̴̴̴̴̴̴̴̴", + expected: "h̸̢̢̢̢̢̢̢̢̢̢e̵̵̵̵̵̵̵̵̵̵l̷̷̷̷̷̷̷̷̷̷l̶̶̶̶̶̶̶̶̶̶o̴̴̴̴̴̴̴̴̴̴", // Preserved as-is (only 50 runes) + }, + { + name: "Long text with combining marks truncated at 128", + input: strings.Repeat("ẽ̸̢̛̖̬͈͉͖͇͈̭̥́̓̌̾͊̊̂̄̍̅̂͌́", 10), // Very long text (260 runes) + expected: "ẽ̸̢̛̖̬͈͉͖͇͈̭̥́̓̌̾͊̊̂̄̍̅̂͌́ẽ̸̢̛̖̬͈͉͖͇͈̭̥́̓̌̾͊̊̂̄̍̅̂͌́ẽ̸̢̛̖̬͈͉͖͇͈̭̥́̓̌̾͊̊̂̄̍̅̂͌́ẽ̸̢̛̖̬͈͉͖͇͈̭̥́̓̌̾͊̊̂̄̍̅̂͌́ẽ̸̢̛̖̬͈͉͖͇͈̭̥́̓̌̾͊̊̂̄̍̅̂", // Truncated at 128 runes + }, + { + name: "Zero-width characters replaced with U+FFFD", + input: "test\u200Bzero\u200Cwidth\u200D\uFEFFchars", + expected: "test�zero�width��chars", + }, + { + name: "RTL/LTR override replaced with U+FFFD", + input: "test\u202Drtl\u202Eltr\u202Aoverride", + expected: "test�rtl�ltr�override", + }, + { + name: "Bidi isolates replaced with U+FFFD", + input: "test\u2066bidi\u2067isolate\u2068text\u2069end", + expected: "test�bidi�isolate�text�end", + }, + { + name: "Control characters replaced with U+FFFD", + input: "test\x00null\x1Fescape\x7Fdelete", + expected: "test�null�escape�delete", + }, + { + name: "Combining marks preserved", + input: "e\u0301\u0302\u0303\u0304\u0305", // e with 5 combining marks + expected: "e\u0301\u0302\u0303\u0304\u0305", // All preserved + }, + { + name: "No truncation at 70 characters", + input: "123456789012345678901234567890123456789012345678901234567890123456789", + expected: "123456789012345678901234567890123456789012345678901234567890123456789", + }, + { + name: "No truncation with Unicode - 70 rockets preserved", + input: strings.Repeat("🚀", 70), + expected: strings.Repeat("🚀", 70), + }, + { + name: "Empty string", + input: "", + expected: "", + }, + { + name: "Only whitespace with control chars", + input: " \t\n ", + expected: "\uFFFD\uFFFD", // Tab and newline become U+FFFD, spaces trimmed + }, + { + name: "Leading and trailing whitespace", + input: " test ", + expected: "test", + }, + { + name: "Complex mix - invisible chars replaced with U+FFFD, Unicode preserved", + input: "kubo/1.0-🚀\u200B h̸̢̏̔ḛ̶̽̀s̵t\u202E-ąęł-中文", + expected: "kubo/1.0-🚀� h̸̢̏̔ḛ̶̽̀s̵t�-ąęł-中文", + }, + { + name: "Emoji with skin tone preserved", + input: "👍🏽", // Thumbs up with skin tone modifier + expected: "👍🏽", // Preserved as-is + }, + { + name: "Mixed scripts preserved", + input: "Hello-你好-مرحبا-Здравствуйте", + expected: "Hello-你好-مرحبا-Здравствуйте", + }, + { + name: "Format characters replaced with U+FFFD", + input: "test\u00ADsoft\u2060word\u206Fnom\u200Ebreak", + expected: "test�soft�word�nom�break", // Soft hyphen, word joiner, etc replaced + }, + { + name: "Complex Unicode text with many combining marks (91 runes, no truncation)", + input: "ț̸̢͙̞̖̏̔ȩ̶̰͓̪͎̱̠̥̳͔̽̀̃̿̌̾̀͗̕̕͜s̵̢̛̖̬͈͉͖͇͈̭̥̃́̓̌̾͊̊̂̄̍̅̂͌́ͅţ̴̯̹̪͖͓̘̊́̑̄̋̈́͐̈́̔̇̄̂́̎̓͛͠ͅ test", + expected: "ț̸̢͙̞̖̏̔ȩ̶̰͓̪͎̱̠̥̳͔̽̀̃̿̌̾̀͗̕̕͜s̵̢̛̖̬͈͉͖͇͈̭̥̃́̓̌̾͊̊̂̄̍̅̂͌́ͅţ̴̯̹̪͖͓̘̊́̑̄̋̈́͐̈́̔̇̄̂́̎̓͛͠ͅ test", // Not truncated (91 < 128) + }, + { + name: "Truncation at 128 characters", + input: strings.Repeat("a", 150), + expected: strings.Repeat("a", 128), + }, + { + name: "Truncation with Unicode at 128", + input: strings.Repeat("🚀", 150), + expected: strings.Repeat("🚀", 128), + }, + { + name: "Private use characters preserved (per spec)", + input: "test\uE000\uF8FF", // Private use area characters + expected: "test\uE000\uF8FF", // Should be preserved + }, + { + name: "U+FFFD replacement for multiple categories", + input: "a\x00b\u200Cc\u202Ed", // control, format chars + expected: "a\uFFFDb\uFFFDc\uFFFDd", // All replaced with U+FFFD + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := cmdutils.CleanAndTrim(tt.input) + assert.Equal(t, tt.expected, result, "CleanAndTrim(%q) = %q, want %q", tt.input, result, tt.expected) + }) + } +} + +func TestCleanAndTrimIdempotent(t *testing.T) { + // Test that applying CleanAndTrim twice gives the same result + inputs := []string{ + "test-ąęćłńóśźż", + "版本-中文测试", + "version-1.0-🚀-🎉", + "h̸e̵l̷l̶o̴ w̸o̵r̷l̶d̴", + "test\u200Bzero\u200Cwidth", + } + + for _, input := range inputs { + once := cmdutils.CleanAndTrim(input) + twice := cmdutils.CleanAndTrim(once) + assert.Equal(t, once, twice, "CleanAndTrim should be idempotent for %q", input) + } +} + +func TestCleanAndTrimSecurity(t *testing.T) { + // Test that all invisible/dangerous characters are removed + tests := []struct { + name string + input string + check func(string) bool + }{ + { + name: "No zero-width spaces", + input: "test\u200B\u200C\u200Dtest", + check: func(s string) bool { + return !strings.Contains(s, "\u200B") && !strings.Contains(s, "\u200C") && !strings.Contains(s, "\u200D") + }, + }, + { + name: "No bidi overrides", + input: "test\u202A\u202B\u202C\u202D\u202Etest", + check: func(s string) bool { + for _, r := range []rune{0x202A, 0x202B, 0x202C, 0x202D, 0x202E} { + if strings.ContainsRune(s, r) { + return false + } + } + return true + }, + }, + { + name: "No control characters", + input: "test\x00\x01\x02\x1F\x7Ftest", + check: func(s string) bool { + for _, r := range s { + if r < 0x20 || r == 0x7F { + return false + } + } + return true + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := cmdutils.CleanAndTrim(tt.input) + assert.True(t, tt.check(result), "Security check failed for %q -> %q", tt.input, result) + }) + } +} diff --git a/test/dependencies/go.mod b/test/dependencies/go.mod index 008cb676a9d..a265621f2c4 100644 --- a/test/dependencies/go.mod +++ b/test/dependencies/go.mod @@ -139,6 +139,7 @@ require ( github.com/ipfs/go-block-format v0.2.2 // indirect github.com/ipfs/go-cid v0.5.0 // indirect github.com/ipfs/go-datastore v0.8.3 // indirect + github.com/ipfs/go-ipfs-cmds v0.15.0 // indirect github.com/ipfs/go-ipfs-redirects-file v0.1.2 // indirect github.com/ipfs/go-ipld-cbor v0.2.1 // indirect github.com/ipfs/go-ipld-format v0.6.2 // indirect diff --git a/test/dependencies/go.sum b/test/dependencies/go.sum index 27501efe95b..b6f1093b34a 100644 --- a/test/dependencies/go.sum +++ b/test/dependencies/go.sum @@ -348,6 +348,8 @@ github.com/ipfs/go-detect-race v0.0.1 h1:qX/xay2W3E4Q1U7d9lNs1sU9nvguX0a7319XbyQ github.com/ipfs/go-detect-race v0.0.1/go.mod h1:8BNT7shDZPo99Q74BpGMK+4D8Mn4j46UU0LZ723meps= github.com/ipfs/go-ipfs-blockstore v1.3.1 h1:cEI9ci7V0sRNivqaOr0elDsamxXFxJMMMy7PTTDQNsQ= github.com/ipfs/go-ipfs-blockstore v1.3.1/go.mod h1:KgtZyc9fq+P2xJUiCAzbRdhhqJHvsw8u2Dlqy2MyRTE= +github.com/ipfs/go-ipfs-cmds v0.15.0 h1:nQDgKadrzyiFyYoZMARMIoVoSwe3gGTAfGvrWLeAQbQ= +github.com/ipfs/go-ipfs-cmds v0.15.0/go.mod h1:VABf/mv/wqvYX6hLG6Z+40eNAEw3FQO0bSm370Or3Wk= github.com/ipfs/go-ipfs-delay v0.0.1 h1:r/UXYyRcddO6thwOnhiznIAiSvxMECGgtv35Xs1IeRQ= github.com/ipfs/go-ipfs-delay v0.0.1/go.mod h1:8SP1YXK1M1kXuc4KJZINY3TQQ03J2rwBG9QfXmbRPrw= github.com/ipfs/go-ipfs-ds-help v1.1.1 h1:B5UJOH52IbcfS56+Ul+sv8jnIV10lbjLF5eOO0C66Nw= diff --git a/test/sharness/t0026-id.sh b/test/sharness/t0026-id.sh index 5d334d863b7..992892a39a6 100755 --- a/test/sharness/t0026-id.sh +++ b/test/sharness/t0026-id.sh @@ -23,13 +23,12 @@ test_id_compute_agent() { fi AGENT_VERSION="$AGENT_VERSION$AGENT_SUFFIX" fi - # limit of 64 characters: https://github.com/ipfs/kubo/pull/9465 - echo -n "$AGENT_VERSION" | head -c 64 + echo "$AGENT_VERSION" } test_expect_success "checking AgentVersion" ' test_id_compute_agent > expected-agent-version && - ipfs id -f "" > actual-agent-version && + ipfs id -f "\n" > actual-agent-version && test_cmp expected-agent-version actual-agent-version ' @@ -47,22 +46,20 @@ test_expect_success "checking and converting ID of a random peer while offline" ' # agent-version-suffix (local, offline) -test_launch_ipfs_daemon --agent-version-suffix=test-suffix-ąę-123456789012345678901234567890 -test_expect_success "checking AgentVersion with suffix (local, only ascii chars)" ' - test_id_compute_agent "test-suffix-_-123456789012345678901234567890" > expected && - ipfs id -f "" > actual && - test_should_not_contain "ą" actual && - test_cmp expected actual +test_launch_ipfs_daemon --agent-version-suffix=test-suffix +test_expect_success "checking AgentVersion with suffix (local)" ' + test_id_compute_agent test-suffix > expected-agent-version && + ipfs id -f "\n" > actual-agent-version && + test_cmp expected-agent-version actual-agent-version ' # agent-version-suffix (over libp2p identify protocol) iptb testbed create -type localipfs -count 2 -init -startup_cluster 2 --agent-version-suffix=test-suffix-identify-óź-123456789012345678901234567890 +startup_cluster 2 --agent-version-suffix=test-suffix-identify test_expect_success "checking AgentVersion with suffix (fetched via libp2p identify protocol)" ' - test_id_compute_agent "test-suffix-identify-_-123456789012345678901234567890" > expected && - ipfsi 1 id "$(ipfsi 0 config Identity.PeerID)" -f "" > actual && - test_should_not_contain "ó" actual && - test_cmp expected actual + ipfsi 0 id -f "\n" > expected-identify-agent-version && + ipfsi 1 id "$(ipfsi 0 config Identity.PeerID)" -f "\n" > actual-libp2p-identify-agent-version && + test_cmp expected-identify-agent-version actual-libp2p-identify-agent-version ' iptb stop @@ -75,7 +72,7 @@ test_expect_success "setting Version.AgentSuffix in config" ' test_launch_ipfs_daemon --agent-version-suffix=ignored-cli-suffix test_expect_success "checking AgentVersion with suffix set via JSON config" ' test_id_compute_agent json-config-suffix > expected-agent-version && - ipfs id -f "" > actual-agent-version && + ipfs id -f "\n" > actual-agent-version && test_cmp expected-agent-version actual-agent-version ' test_kill_ipfs_daemon diff --git a/version.go b/version.go index c53c38bb108..c1d227b6150 100644 --- a/version.go +++ b/version.go @@ -2,9 +2,9 @@ package ipfs import ( "fmt" - "regexp" "runtime" - "strings" + + "github.com/ipfs/kubo/core/commands/cmdutils" ) // CurrentCommit is the current git commit, this is set as a ldflag in the Makefile. @@ -18,8 +18,6 @@ const ApiVersion = "/kubo/" + CurrentVersionNumber + "/" //nolint // RepoVersion is the version number that we are currently expecting to see. const RepoVersion = 17 -const maxVersionLen = 64 - // GetUserAgentVersion is the libp2p user agent used by go-ipfs. // // Note: This will end in `/` when no commit is available. This is expected. @@ -31,35 +29,13 @@ func GetUserAgentVersion() string { } userAgent += userAgentSuffix } - return TrimVersion(userAgent) + return cmdutils.CleanAndTrim(userAgent) } var userAgentSuffix string -// nonPrintableASCII matches characters outside the printable ASCII range (space through tilde) -var nonPrintableASCII = regexp.MustCompile(`[^\x20-\x7E]+`) - func SetUserAgentSuffix(suffix string) { - userAgentSuffix = TrimVersion(suffix) -} - -// TrimVersion sanitizes version strings to contain only printable ASCII -// limited to maxVersionLen characters. Non-printable characters are replaced with underscores. -func TrimVersion(version string) string { - version = strings.TrimSpace(version) - if version == "" { - return "" - } - - // Replace runs of non-printable ASCII with single underscore - sanitized := nonPrintableASCII.ReplaceAllString(version, "_") - - // Truncate to max length (safe since we have ASCII-only) - if len(sanitized) > maxVersionLen { - sanitized = sanitized[:maxVersionLen] - } - - return sanitized + userAgentSuffix = cmdutils.CleanAndTrim(suffix) } type VersionInfo struct {