public inbox for nncp-devel@lists.cypherpunks.su
Atom feed
* NNCP path traversal attack.
@ 2025-09-19  4:46 Eugene Medvedev
  2025-09-19  6:28 ` Jonathan Lane
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Eugene Medvedev @ 2025-09-19  4:46 UTC (permalink / raw)
  To: nncp-devel

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

As it currently stands, NNCP is vulnerable to path traversal attacks with
freq and file functions: Despite the requirement for both to supply full path
in configuration, both types of packets will accept and act upon paths
containing
"..". Most obviously, this allows one to request any file NNCP has access to,
like its own configuration file with the private keys in it.
Likewise, a sent file can break out of the incoming directory in the same manner
and be written anywhere on the system that the user can write to.

The included patch is my take on dealing with this by by limiting path
traversal to
below the configured full path. It does nothing about, e.g., symlinks,
and I'm not sure anything should be done about those.

I can't claim to understand the codebase sufficiently to have caught
all the ways
this can happen, however.

-- 
Eugene Medvedev

[-- Attachment #2: traversal.patch --]
[-- Type: text/x-patch, Size: 2108 bytes --]

As it currently stands, NNCP is vulnerable to path traversal attacks with
freq and file functions: Despite the requirement for both to supply full path
in configuration, both types of packets will accept and act upon paths containing
"..". Most obviously, this allows one to request any file NNCP has access to,
like its own configuration file with the private keys in it.
Likewise, a sent file can break out of the incoming directory in the same manner
and be written anywhere on the system that the user can write to.

This patch is my take on dealing with this by by limiting path traversal to
below the configured full path. It does nothing about, e.g., symlinks,
and I'm not sure anything should be done about those.

diff -ruN nncp-8.11.0/src/toss.go nncp-8.11.0-patched/src/toss.go
--- nncp-8.11.0/src/toss.go	1970-01-01 03:00:00.000000000 +0300
+++ nncp-8.11.0-patched/src/toss.go	2025-09-18 23:26:07.988137948 +0300
@@ -312,6 +312,17 @@
 			return err
 		}
 		dir := filepath.Join(*incoming, path.Dir(dst))
+		if !strings.HasPrefix(dir, *incoming) {
+			err = errors.New("incoming path traversal")
+			ctx.LogE("rx-traversal", les, err, func(les LEs) string {
+				return fmt.Sprintf(
+					"Tossing file %s/%s (%s): %s: traversal",
+					sender.Name, pktName,
+					humanize.IBytes(pktSize), dst,
+				)
+			})
+			return err
+		}
 		if err = os.MkdirAll(dir, os.FileMode(0777)); err != nil {
 			ctx.LogE("rx-mkdir", les, err, func(les LEs) string {
 				return fmt.Sprintf(
@@ -542,11 +553,26 @@
 			)
 			return err
 		}
+		srcPath := filepath.Join(*freqPath, src)
+		if !strings.HasPrefix(srcPath, *freqPath) {
+			err = errors.New("freqing path traversal")
+			ctx.LogE(
+				"rx-no-freq", les, err,
+				func(les LEs) string {
+					return fmt.Sprintf(
+						"Tossing freq %s/%s (%s): %s -> %s",
+						sender.Name, pktName,
+						humanize.IBytes(pktSize), src, dst,
+					)
+				},
+			)
+			return err
+		}
 		if !opts.DryRun {
 			err = ctx.TxFile(
 				sender,
 				pkt.Nice,
-				filepath.Join(*freqPath, src),
+				srcPath,
 				dst,
 				sender.FreqChunked,
 				sender.FreqMinSize,

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NNCP path traversal attack.
  2025-09-19  4:46 NNCP path traversal attack Eugene Medvedev
@ 2025-09-19  6:28 ` Jonathan Lane
  2025-09-19  6:43   ` Eugene Medvedev
  2025-09-19 12:02 ` John Goerzen
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Jonathan Lane @ 2025-09-19  6:28 UTC (permalink / raw)
  To: Eugene Medvedev, nncp-devel

On Thu Sep 18, 2025 at 9:46 PM PDT, Eugene Medvedev wrote:
> The included patch is my take on dealing with this by by limiting path
> traversal to
> below the configured full path. It does nothing about, e.g., symlinks,
> and I'm not sure anything should be done about those.

The usual protection against symlink traversal is running services in a
chroot jail, Docker, or some other imposed filesystem boundary.  I'm not
sure there's a good source-level fix for this that runs everywhere NNCP
does.

-- 
Jonathan Lane



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NNCP path traversal attack.
  2025-09-19  6:28 ` Jonathan Lane
@ 2025-09-19  6:43   ` Eugene Medvedev
  0 siblings, 0 replies; 9+ messages in thread
From: Eugene Medvedev @ 2025-09-19  6:43 UTC (permalink / raw)
  To: Jonathan Lane; +Cc: nncp-devel

On Fri, 19 Sept 2025 at 09:29, Jonathan Lane <jon@borg•moe> wrote:

> The usual protection against symlink traversal is running services in a
> chroot jail, Docker, or some other imposed filesystem boundary.  I'm not
> sure there's a good source-level fix for this that runs everywhere NNCP
> does.

I'm not sure symlink traversal is a problem per se: You can't introduce a
symlink with nncp-file, while whatever symlinks exist are presumably where
the owner of the node wanted them.

Plain path traversal, where you have a freq in "/fileshare/whatever" and
then an incoming packet requests "../../etc/nncp.hjson" and gets it,
definitely is a problem that can be patched away, and hopefully my
patch is sufficient. I don't think Docker or chroot jails can cover that,
because every time you toss you have to read nncp.json...

-- 
Eugene Medvedev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NNCP path traversal attack.
  2025-09-19  4:46 NNCP path traversal attack Eugene Medvedev
  2025-09-19  6:28 ` Jonathan Lane
@ 2025-09-19 12:02 ` John Goerzen
  2025-09-19 12:11   ` Eugene Medvedev
  2025-09-19 12:04 ` John Goerzen
  2025-09-19 13:25 ` Sergey Matveev
  3 siblings, 1 reply; 9+ messages in thread
From: John Goerzen @ 2025-09-19 12:02 UTC (permalink / raw)
  To: Eugene Medvedev; +Cc: nncp-devel

Thank you for noticing and patching this, Eugene.

I have temporarily disabled freq on quux until I get it patched.

I will also work on getting a patched Debian package out.  I am pretty
busy through Sunday though, so it may be a couple of days.  I'm not sure
if Sergey is available; if not, I'll post a patched release on a git
system somewhere for ease of installation.

Question: do you believe there is also an absolute path (path beginning
with / ) weakness?  It looks like your patch would account for that also
though.

Thanks,

John

On Fri, Sep 19 2025, Eugene Medvedev wrote:

> As it currently stands, NNCP is vulnerable to path traversal attacks with
> freq and file functions: Despite the requirement for both to supply full path
> in configuration, both types of packets will accept and act upon paths
> containing
> "..". Most obviously, this allows one to request any file NNCP has access to,
> like its own configuration file with the private keys in it.
> Likewise, a sent file can break out of the incoming directory in the same manner
> and be written anywhere on the system that the user can write to.
>
> The included patch is my take on dealing with this by by limiting path
> traversal to
> below the configured full path. It does nothing about, e.g., symlinks,
> and I'm not sure anything should be done about those.
>
> I can't claim to understand the codebase sufficiently to have caught
> all the ways
> this can happen, however.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NNCP path traversal attack.
  2025-09-19  4:46 NNCP path traversal attack Eugene Medvedev
  2025-09-19  6:28 ` Jonathan Lane
  2025-09-19 12:02 ` John Goerzen
@ 2025-09-19 12:04 ` John Goerzen
  2025-09-19 12:31   ` Eugene Medvedev
  2025-09-19 13:25 ` Sergey Matveev
  3 siblings, 1 reply; 9+ messages in thread
From: John Goerzen @ 2025-09-19 12:04 UTC (permalink / raw)
  To: Eugene Medvedev; +Cc: nncp-devel

On symlinks:

One can at least test if the resulting path is a symlink, and refuse to
process it if so.  That may possibly have a race condition with other
things on the system, but since nncp-toss is single-threaded, probably
not there.

One vulnerability could be if incoming and freq are the same path.  I'm
not sure if nncp-file will pack up symlinks when asked to send an entire
directory; if so, that could be an issue.  But I suspect that most of
the time, if somebody is using both freq and incoming, they'd be
separate directories.


On Fri, Sep 19 2025, Eugene Medvedev wrote:

> As it currently stands, NNCP is vulnerable to path traversal attacks with
> freq and file functions: Despite the requirement for both to supply full path
> in configuration, both types of packets will accept and act upon paths
> containing
> "..". Most obviously, this allows one to request any file NNCP has access to,
> like its own configuration file with the private keys in it.
> Likewise, a sent file can break out of the incoming directory in the same manner
> and be written anywhere on the system that the user can write to.
>
> The included patch is my take on dealing with this by by limiting path
> traversal to
> below the configured full path. It does nothing about, e.g., symlinks,
> and I'm not sure anything should be done about those.
>
> I can't claim to understand the codebase sufficiently to have caught
> all the ways
> this can happen, however.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NNCP path traversal attack.
  2025-09-19 12:02 ` John Goerzen
@ 2025-09-19 12:11   ` Eugene Medvedev
  0 siblings, 0 replies; 9+ messages in thread
From: Eugene Medvedev @ 2025-09-19 12:11 UTC (permalink / raw)
  To: John Goerzen; +Cc: nncp-devel

On Fri, 19 Sept 2025 at 15:02, John Goerzen <jgoerzen@complete•org> wrote:

> Question: do you believe there is also an absolute path (path beginning
> with / ) weakness?  It looks like your patch would account for that also
> though.

The code explicitly checked for non-relative path in case of both freq
and file even before my patch, so fortunately that wasn't a concern.

-- 
Eugene Medvedev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NNCP path traversal attack.
  2025-09-19 12:04 ` John Goerzen
@ 2025-09-19 12:31   ` Eugene Medvedev
  0 siblings, 0 replies; 9+ messages in thread
From: Eugene Medvedev @ 2025-09-19 12:31 UTC (permalink / raw)
  To: John Goerzen; +Cc: nncp-devel

On Fri, 19 Sept 2025 at 15:04, John Goerzen <jgoerzen@complete•org> wrote:

> One can at least test if the resulting path is a symlink, and refuse to
> process it if so.  That may possibly have a race condition with other
> things on the system, but since nncp-toss is single-threaded, probably
> not there.

Consider the following usage pattern:

I have "/mnt/extradrive/distros" where I keep ISO images of linux distros.
I also have "/mnt/otherdrive/music" where I keep music I made.
I wish to share both directories, but I can't just put "/mnt" up for freq:
that would give people access to "/mnt/thirdddrive/mildsecret" that I
need world readable for my system to function, but don't want to let
outside users see.

So instead I create "/var/spool/nncp-freq", put that up for freq, and in it,
I create two symlinks: "distros" pointing to "/mnt/extradrive/distros" and
"music" pointing to "/mnt/otherdrive/music"

As of right now, this usage pattern works: from the freq point of view,
making requests for "node:distros/foo" and "node:music/bar" returns
the contents of "/mnt/extradrive/distros/foo" and
"/mnt/otherdrive/music/bar" respectively. If "foo" or "bar" are directories,
or symlinks to directories, nncp will pax them and send the archive,
which makes freqs so much more useful.

I believe this is quite a legitimate usage pattern, which is why I'm saying
that symlinks are probably not a problem, but rather a solution.
Given this example, my patch prevents requests for
"node:../../../mnt/thirddrive/mildsecret" from working, but otherwise does
not change this behavior.

os.Root, available since Go 1.24, would be a lot more comprehensive
than the simple check my patch does, and I think it would catch the
symlinks too, if I'm reading the docs right, but it would also be more
restrictive and prevent this usage pattern entirely, necessitating
more complex things like multiple freq directories, etc.

> One vulnerability could be if incoming and freq are the same path.

Which is something that should probably be documented as a
configuration to be avoided, because no amount of coding can
guard against deliberate misconfiguration.

-- 
Eugene Medvedev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NNCP path traversal attack
  2025-09-19  4:46 NNCP path traversal attack Eugene Medvedev
                   ` (2 preceding siblings ...)
  2025-09-19 12:04 ` John Goerzen
@ 2025-09-19 13:25 ` Sergey Matveev
  2025-09-19 13:30   ` Eugene Medvedev
  3 siblings, 1 reply; 9+ messages in thread
From: Sergey Matveev @ 2025-09-19 13:25 UTC (permalink / raw)
  To: nncp-devel

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

Greetings!

*** Eugene Medvedev [2025-09-19 07:46]:
>As it currently stands, NNCP is vulnerable to path traversal attacks with
>freq and file functions: Despite the requirement for both to supply full path
>in configuration, both types of packets will accept and act upon paths
>containing "..".

Unfortunately it is so indeed. Shame on me for missing that check!
Thanks for the patch! I merged it into develop branch. May I add you to
the THANKS file? If yes, should I add your email there? After that I
will make a new release.

-- 
Sergey Matveev (http://www.stargrave.org/)
LibrePGP: 12AD 3268 9C66 0D42 6967  FD75 CB82 0563 2107 AD8A

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NNCP path traversal attack
  2025-09-19 13:25 ` Sergey Matveev
@ 2025-09-19 13:30   ` Eugene Medvedev
  0 siblings, 0 replies; 9+ messages in thread
From: Eugene Medvedev @ 2025-09-19 13:30 UTC (permalink / raw)
  To: nncp-devel

On Fri, 19 Sept 2025 at 16:26, Sergey Matveev <stargrave@stargrave•org> wrote:

> Thanks for the patch! I merged it into develop branch. May I add you to
> the THANKS file? If yes, should I add your email there? After that I
> will make a new release.

Everyone likes being thanked, though I'd prefer my email stay out of it. :)

-- 
Eugene Medvedev

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-09-19 13:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-19  4:46 NNCP path traversal attack Eugene Medvedev
2025-09-19  6:28 ` Jonathan Lane
2025-09-19  6:43   ` Eugene Medvedev
2025-09-19 12:02 ` John Goerzen
2025-09-19 12:11   ` Eugene Medvedev
2025-09-19 12:04 ` John Goerzen
2025-09-19 12:31   ` Eugene Medvedev
2025-09-19 13:25 ` Sergey Matveev
2025-09-19 13:30   ` Eugene Medvedev