request server: handle relative symlinks #526

georgmu · 2022-09-29T10:02:23Z

The request server implementation does not handle symlinks correctly. If the link target is relative it is rewritten to an absolute path. The first commit fixes that.

The second commit is just adding a testcase for linking to non-existent files.

drakkan · 2022-09-29T10:41:18Z

I don't have time to test this change now (I'll do it later today or this weekend), I think this change will allow symlinks outside the base directory. Applications enforcing a chroot such as SFTPGo should check the symlink target and normalize it themselves after this change. This is acceptable but maybe should be documented as a backward incompatible change.

georgmu · 2022-09-29T13:40:45Z

I just did a basic test with SFTPGo: Escaping the chroot (using cd or get) didn't seem to work - fine so far.

SFTPGo is restricting the link target of the symlink command pretty strong - but since symlinks could exist from other sources (directly in filesystem using ln)

But yes, SFTPGo would need to change its implementation to handle relative symlinks correctly.
Maybe I should file a sepearate bug in SFTPGo, since symlink handling of relative paths is pretty easy to trigger using the command-line sftp command:

$ echo bar > bar
$ sftp -P 2022 user@localhost
Connected to localhost.
sftp> mkdir foo
sftp> cd foo
sftp> put bar
Uploading bar to /foo/bar
bar
sftp> rename bar renamed-bar # rename here if local pwd is same as home directory of user
sftp> symlink renamed-bar linked-bar
sftp> ls -l
lrwxrwxrwx    1 1000     1000           23 Sep 29 15:34 linked-bar
-rw-r--r--    1 1000     1000            4 Sep 29 15:33 renamed-bar
sftp> get linked-bar
Fetching /foo/linked-bar to linked-bar
Couldn't stat remote file: No such file or directory
sftp> ^D
$ ls -l foo/
insgesamt 8
lrwxrwxrwx. 1 user user 23 29. Sep 15:34 linked-bar -> /home/user/renamed-bar
-rw-r--r--. 1 user user  4 29. Sep 15:33 renamed-bar

Symlink linked-bar should either be relative (best option) or be /home/user/foo/renamed-bar (but this has the drawback of destroying the link by renaming directory foo to foo2)

georgmu · 2022-09-29T14:00:38Z

Just as a note: The last comment and tests were about the current SFTPGo implementation without the relative symlink patch applied to sftp.

drakkan · 2022-09-29T14:03:37Z

Just as a note: The last comment and tests were about the current SFTPGo implementation without the relative symlink patch applied to sftp.

Yes, I noticed this morning, after your initial comment that relative symlinks are not handled correctly in SFTPGo. I'll give a look this weekend, thanks

puellanivis · 2022-09-29T14:10:50Z

request-server_test.go

@@ -590,6 +604,17 @@ func TestRequestSymlink(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, []byte("hello"), content)

+	fi, err = r.lfetch("/link-to-non-existent-file")
+	require.NoError(t, err)
+	assert.True(t, fi.Mode()&os.ModeSymlink == os.ModeSymlink)


When using assert.True it can be very useful to put in some sort of explaining communication about what this is actually testing, otherwise the test just fails with expected: true \n got: false which is pretty much not helpful at all.

However, I suggest instead, skipping the whole assert library† and just write the test clearly:

if fi.Mode() & os.ModeSymlink != os.ModeSymlink { t.Error("expected mode to be a symlink, but was: ", fi.Mode()) }

†: I recommend skiping these assert libraries entirely and always, but we’re already using it heavily here. There is a reason why Golang doesn’t have a standard library assert package, and it’s because it leads people to be careless about their tests, and obfuscates how the code should be used. Namely, test code should be as close to example code as possible. (Bonus: if writing real production-quality code in your tests is cumbersome, it’s a signal to fix your API, not wallpaper over the burden of writing the tests.

this was just a copy of the symlink test from the same testcase. I will rework this one and the others aswell

puellanivis · 2022-09-29T14:12:41Z

request-server_test.go

+	assert.True(t, fi.Mode()&os.ModeSymlink == os.ModeSymlink)
+
+	_, err = r.fetch("/link-to-non-existent-file")
+	require.True(t, os.IsNotExist(err))


Same here, after adding the explanatory details we’re not getting anything out of the require package:

if !os.IsNotExist(err) { t.Fatal("expected file to not exist, but it did") }

drakkan · 2022-09-29T15:44:42Z

I think this was broken by me here. @georgmu can you please revert that commit (or revert to the previous behavior) and confirm that your use case is working?

Ignore this comment. Even before my change pkg.getPath get cleaned in the NewRequest method.

drakkan · 2022-09-29T17:07:19Z

I've done some testing and this patch looks good to me. I think we just need to document somewhere that Filepath can be relative for symbolic links now. @puellanivis do you have any suggestions other than the above for test cases?

drakkan · 2022-09-29T18:58:29Z

@georgmu please add some test cases for relative links from a dir different from /, something like this:

sftp> pwd
Remote working directory: /dir
sftp> symlink link1.txt sub/file.txt
sftp> symlink sub/link2.txt file.txt

Thanks

puellanivis · 2022-09-29T19:50:24Z

I've done some testing and this patch looks good to me. I think we just need to document somewhere that Filepath can be relative for symbolic links now. @puellanivis do you have any suggestions other than the above for test cases?

Nope, not really. Tried my hardest, and only the test code was something I could see anything at all in.

georgmu · 2022-09-30T07:41:30Z

I will add some more testcases, also the one mentioned in my comment about moving the parent directory. I will also try the testcases with 98b35dc reverted (just to check that it worked before).

georgmu · 2022-09-30T07:56:33Z

sftp> pwd
Remote working directory: /dir
sftp> symlink link1.txt sub/file.txt
sftp> symlink sub/link2.txt file.txt

just to make sure what these commands do as checked with sftp CLI client and standard sftp server:

symlink link1.txt sub/file.txt will create symlink /dir/sub/file.txt
- /dir/sub has to exist
- link target: link1.txt (which would effectively be file /dir/sub/link1.txt)
symlink sub/link2.txt file.txt will create symlink /dir/file.txt
- /dir/sub does not have to exist
- link target: sub/link2.txt (which would effectively be file /dir/sub/link2.txt)

I would reverse the names so that the symlinks are named link*.txt and the files they are pointing to named file*.txt

drakkan · 2022-09-30T11:21:14Z

sftp> pwd
Remote working directory: /dir
sftp> symlink link1.txt sub/file.txt
sftp> symlink sub/link2.txt file.txt
just to make sure what these commands do as checked with sftp CLI client and standard sftp server:

symlink link1.txt sub/file.txt will create symlink /dir/sub/file.txt

/dir/sub has to exist

link target: link1.txt (which would effectively be file /dir/sub/link1.txt)

symlink sub/link2.txt file.txt will create symlink /dir/file.txt

/dir/sub does not have to exist

link target: sub/link2.txt (which would effectively be file /dir/sub/link2.txt)

I would reverse the names so that the symlinks are named link*.txt and the files they are pointing to named file*.txt

Yes, reverse the names, thanks

georgmu · 2022-09-30T12:28:14Z

@georgmu please add some test cases for relative links from a dir different from /, something like this:
sftp> pwd
Remote working directory: /dir

Thanks

As I understand sftp, pwd is solely a client feature keeping track of the current directory. So the unit tests can only use /dir/sub/... etc. ...

drakkan · 2022-09-30T12:45:10Z

@georgmu please add some test cases for relative links from a dir different from /, something like this:
sftp> pwd
Remote working directory: /dir

Thanks
As I understand sftp, pwd is solely a client feature keeping track of the current directory. So the unit tests can only use /dir/sub/... etc. ...

Yes, you are right, you already included a test like this:

err = p.cli.Symlink("f1", "/subdir/linked_f1")
require.NoError(t, err)

I added such tests to SFTPGo because I have to handle other things there

georgmu · 2022-09-30T13:43:32Z

One more thing here:
I was also testing Readlink() results and they are manipulated as well for absolute links.

From my understanding, a symlink is just a textfile whose content gets interpreted by the virtual file system. So if I write /foo/bar into a textfile I expect the content to be /foo/bar whenever I read it.

chroot'ing does not alter the content ("target") of the link, only the interpretation of the content.

lets assume a symlink /subdir/link.txt with link target /foo/bar
when reading /subdir/link.txt the content of /foo/bar is returned
when in a chroot /subdir, reading /link.txt returns the content of /foo/bar in the chroot, which is effectively /subdir/foo/bar from outside of the chroot

This is how symlinks work and I think the library should do the same: not interpreting the link target.

What a chroot-in-software (what SFTPGo does) should do sanity checks whenever reading a link (like interpreting absolute links as links from the chroot and check that relative links do not escape the chroot), but the library should return them as is.

drakkan · 2022-09-30T14:21:03Z

One more thing here: I was also testing Readlink() results and they are manipulated as well for absolute links.

From my understanding, a symlink is just a textfile whose content gets interpreted by the virtual file system. So if I write /foo/bar into a textfile I expect the content to be /foo/bar whenever I read it.

chroot'ing does not alter the content ("target") of the link, only the interpretation of the content.

lets assume a symlink /subdir/link.txt with link target /foo/bar

when reading /subdir/link.txt the content of /foo/bar is returned

when in a chroot /subdir, reading /link.txt returns the content of /foo/bar in the chroot, which is effectively /subdir/foo/bar from outside of the chroot

This is how symlinks work and I think the library should do the same: not interpreting the link target.

The library should pass the cleaned path for readlink requests, the request server implementations (SFTPGo or others) can handle the request according to their own logic, which could be the one you describe or a different one

What a chroot-in-software (what SFTPGo does) should do sanity checks whenever reading a link (like interpreting absolute links as links from the chroot and check that relative links do not escape the chroot), but the library should return them as is.

SFTPGo resolves the link and if it is outside the chroot directory it will return an error like this:

"level":"error","time":"2022-09-30T15:56:24.580","sender":"osfs","connection_id":"43bea5101a92edede8edbeebdec1ded65406186c4b875013eed73d1c332de9c5","message":"Invalid path resolution, path \"/tmp/test_home/foo/bar\" original path \"/link.txt\" resolved \"/tmp/test_home/subdir/link.txt\" err: Path resolution error: path \"/tmp/test_home/foo/bar\" is not inside \"/tmp/test_home/subdir\""}

georgmu · 2022-09-30T14:31:00Z

The library should pass the cleaned path for readlink requests

Currently, it returns a relative path, even if an absolute path is generated.

p.cli.Symlink("/foo", "/bar")
...
p.cli.Readlink("/bar") // <- "foo" instead of "/foo"

I just checked the code: openssh sftp-server also just returns the content of readlink(2) and ln does not clean the symlink when generating one (ln -s /a/b/../c foo will have exactly /a/b/../c as target instead of cleaned /a/c.

georgmu · 2022-09-30T14:41:29Z

The problem with ReadLink is here:

sftp/request.go

Line 584 in 3aa4378

filename := finfo[0].Name()

file.Name() only returns the base name of the file (as specified in docs for os.FileInfo), but this is wrong here.

Solution would be to depend on something other than os.FileInfo here or return a custom Sys() result.

drakkan · 2022-09-30T15:04:36Z

ah ok, now I better understand what you mean. In SFTPGo I use this workaround

https://github.com/drakkan/sftpgo/blob/main/internal/sftpd/handler.go#L263

I have custom fileinfo and for the Readlink case I set fullName to true and so Name() returns the full path

https://github.com/drakkan/sftpgo/blob/1e21aa945338dae031fcf49495f2f802fcc0ffd6/internal/vfs/fileinfo.go#L37

I agree we should fix this issue in a better way (I use the same hack also for FTP).

drakkan · 2022-09-30T15:18:49Z

The library should pass the cleaned path for readlink requests

Currently, it returns a relative path, even if an absolute path is generated.
p.cli.Symlink("/foo", "/bar")
...
p.cli.Readlink("/bar") // <- "foo" instead of "/foo" 
I just checked the code: openssh sftp-server also just returns the content of readlink(2) and ln does not clean the symlink when generating one (ln -s /a/b/../c foo will have exactly /a/b/../c as target instead of cleaned /a/c.

As for the server side path normalization, the library always cleaned paths (I'm not the original author), the only expection is the request.Filepath for symlinks introduced in this PR. I suggest to discuss this in a separate issue/PR, thanks

georgmu · 2022-09-30T15:46:31Z

I just uploaded a solution for both symlink and readlink stuff with extended test cases.

The readlink fix is a bit hacky and requires some documentation and/or another type.

drakkan · 2022-09-30T16:11:15Z

I'm not sure about the readlink fix. You have to use a custom fileinfo implementing the new LinkReader interface.

Readlink is currently broken, I think we can break backward compatibility for something that does not work or that works with hacks like the one I used in SFTPGo. @puellanivis what do you think about? (I already know you disagree 😄 )

drakkan · 2022-09-30T17:26:51Z

@georgmu I'm now using your initial patch in SFTPGo

drakkan/sftpgo@0e8c41b#diff-d48bed73cb243b5300108a318eea6494652d40093322e498e5fd5d098725ef33
drakkan@e8c89af

thanks!

drakkan · 2022-10-01T07:43:18Z

We could do something like this:

diff --git a/request-interfaces.go b/request-interfaces.go
index 2e5ee6b..ab5f5a1 100644
--- a/request-interfaces.go
+++ b/request-interfaces.go
@@ -94,7 +94,7 @@ type LstatFileLister interface {
 //
 // Up to v1.13.5 the signature for the RealPath method was:
 //
-// RealPath(string) string
+// # RealPath(string) string
 //
 // we have added a legacyRealPathFileLister that implements the old method
 // to ensure that your code does not break.
@@ -104,6 +104,13 @@ type RealPathFileLister interface {
        RealPath(string) (string, error)
 }
 
+// ReadlinkFileLister is a FileLister that implements the Readlink method.
+// TODO: complete docs
+type ReadlinkFileLister interface {
+       FileLister
+       Readlink(string) (string, error)
+}
+
 // This interface is here for backward compatibility only
 type legacyRealPathFileLister interface {
        FileLister
diff --git a/request.go b/request.go
index 6a7e6cf..4db3f54 100644
--- a/request.go
+++ b/request.go
@@ -295,7 +295,25 @@ func (r *Request) call(handlers Handlers, pkt requestPacket, alloc *allocator, o
                return filecmd(handlers.FileCmd, r, pkt)
        case "List":
                return filelist(handlers.FileList, r, pkt)
-       case "Stat", "Lstat", "Readlink":
+       case "Stat", "Lstat":
+               return filestat(handlers.FileList, r, pkt)
+       case "Readlink":
+               if readlinkFileLister, ok := handlers.FileList.(ReadlinkFileLister); ok {
+                       resolved, err := readlinkFileLister.Readlink(r.Filepath)
+                       if err != nil {
+                               return statusFromError(pkt.id(), err)
+                       }
+                       return &sshFxpNamePacket{
+                               ID: pkt.id(),
+                               NameAttrs: []*sshFxpNameAttr{
+                                       {
+                                               Name:     resolved,
+                                               LongName: resolved,
+                                               Attrs:    emptyFileStat,
+                                       },
+                               },
+                       }
+               }
                return filestat(handlers.FileList, r, pkt)
        default:
                return statusFromError(pkt.id(), fmt.Errorf("unexpected method: %s", r.Method))

@puellanivis, @georgmu do you have other/better suggestions? thanks

puellanivis

Not sure on the other questions being asked. I’m a little sick right now, and finding it difficult to follow things well enough.

puellanivis · 2022-10-01T19:29:58Z

request.go

+			if f, err := rl.ReadLink(); err != nil {
+				return statusFromError(pkt.id(), err)
+			} else {
+				filename = f
+			}


This is better written as:

f, err := rl.ReadLink() if err != nil { return statusFromError(pkt.id(), err) } filename = f

We could do something like this:

diff --git a/request-interfaces.go b/request-interfaces.go index 2e5ee6b..ab5f5a1 100644 --- a/request-interfaces.go +++ b/request-interfaces.go @@ -94,7 +94,7 @@ type LstatFileLister interface { // // Up to v1.13.5 the signature for the RealPath method was: // -// RealPath(string) string +// # RealPath(string) string // // we have added a legacyRealPathFileLister that implements the old method // to ensure that your code does not break. @@ -104,6 +104,13 @@ type RealPathFileLister interface { RealPath(string) (string, error) } +// ReadlinkFileLister is a FileLister that implements the Readlink method. +// TODO: complete docs +type ReadlinkFileLister interface { + FileLister + Readlink(string) (string, error) +} + // This interface is here for backward compatibility only type legacyRealPathFileLister interface { FileLister diff --git a/request.go b/request.go index 6a7e6cf..4db3f54 100644 --- a/request.go +++ b/request.go @@ -295,7 +295,25 @@ func (r *Request) call(handlers Handlers, pkt requestPacket, alloc *allocator, o return filecmd(handlers.FileCmd, r, pkt) case "List": return filelist(handlers.FileList, r, pkt) - case "Stat", "Lstat", "Readlink": + case "Stat", "Lstat": + return filestat(handlers.FileList, r, pkt) + case "Readlink": + if readlinkFileLister, ok := handlers.FileList.(ReadlinkFileLister); ok { + resolved, err := readlinkFileLister.Readlink(r.Filepath) + if err != nil { + return statusFromError(pkt.id(), err) + } + return &sshFxpNamePacket{ + ID: pkt.id(), + NameAttrs: []*sshFxpNameAttr{ + { + Name: resolved, + LongName: resolved, + Attrs: emptyFileStat, + }, + }, + } + } return filestat(handlers.FileList, r, pkt) default: return statusFromError(pkt.id(), fmt.Errorf("unexpected method: %s", r.Method))

@puellanivis, @georgmu do you have other/better suggestions? thanks

I think this sounds like a good solution and is done in the same way as the LstatFileLister.

Another option would be a bigger rewrite but this would brake existing code: Make Handlers an Interface (it is used way in most cases anyway where FileGet/FilePut/FileCmd/FileList all point to the same implementation) and make the now optional interfaces mandatory with a hint on what to return when support is not necessary. Go is lacking features like default implementations for interfaces like in rust...

Another option would be a bigger rewrite but this would brake existing code: Make Handlers an Interface (it is used way in most cases anyway where FileGet/FilePut/FileCmd/FileList all point to the same implementation) and make the now optional interfaces mandatory with a hint on what to return when support is not necessary. Go is lacking features like default implementations for interfaces like in rust...

This will require a jump to V2. But this is something that we really do need to get done. Default implementations can be done by allowing embedded types that implement the default. See protobuf and their “UnimplementedXYServer”.

We can also cover default implementations through interface extensions as we have been doing a few of the newer implementations.

I understand that it is hard to shift out of a familiar paradigm where rust provides for default implementations, but there exists an equivalent in Go. You just have to often think about things from a different perspective.

drakkan · 2022-10-05T08:05:49Z

I try to summarize:

the initial patch from @georgmu (georgmu@9bc346c) looks ok to me and to @puellanivis
while writing test cases @georgmu discovered a bug in Readlink handling that I was aware of but I was too lazy to report and fix in the past (I applied a hacky workaround in SFTPGo)
to fix Readlink in a backward compatible way I proposed to add this optional interface:

type ReadlinkFileLister interface {
	FileLister
	Readlink(string) (string, error)
}

we'll break backward compatibility in v2.

If we all agree with this approach we can add the ReadlinkFileLister interface.

drakkan · 2022-10-07T10:31:40Z

@georgmu are you interested to add the ReadlinkFileLister interface as discussed? I'm busy with many other things and I don't care about the authorship of the code pasted above. Thanks

georgmu · 2022-10-07T10:32:58Z

@georgmu are you interested to add the ReadlinkFileLister interface as discussed? I'm busy with many other things and I don't care about the authorship of the code pasted above. Thanks

Sure. I will prepare the commits.

georgmu · 2022-10-11T15:44:25Z

I prepared a commit for the ReadlinkFileLister. I am not the best when it comes to documentation. If I should extend the notes, please just give me a hint on what to improve.

drakkan · 2022-10-12T10:30:33Z

I prepared a commit for the ReadlinkFileLister. I am not the best when it comes to documentation. If I should extend the notes, please just give me a hint on what to improve.

Thanks! You are much better than me at writing docs 😄 The PR LGTM. Maybe you can also test returning an error from a readlink request. There is no coverage:

drakkan

Thanks! This PR is ok for me. I'll wait a few more days to see if @puellanivis has any other suggestions before merging this patch

puellanivis

That I am providing feedback only on documentation and string format verbs is a testament to the code being pretty great. 👍

request-server_test.go

puellanivis · 2022-10-13T00:36:30Z

request-interfaces.go

@@ -74,6 +74,10 @@ type StatVFSFileCmder interface {
 // FileLister should return an object that fulfils the ListerAt interface
 // Note in cases of an error, the error text will be sent to the client.
 // Called for Methods: List, Stat, Readlink
+// For Readlink, Filelist is limited to os.FileInfo response. For a more


I might recommend a paragraph break here. The added whitespace makes the separation of the purpose of the two comments more clear.

Also, I know the existing text looks like it’s using a 80 column break, each of the three existing lines is a distinct individual semantic statement. In Golang, you don’t want to be breaking up lines just because you’re fitting a column width, but rather because they’re natural breaks. So more https://sembr.org/

So:

// For Readlink, Filelist is limited to os.FileInfo response. // For more sophisticated Readlink response, please implement ReadlinkFileLister. // If ReadlinkFileLister is implemented, its Readlink method is called for method Readlink instead of calling Filelist.

I’d probably also suggest being more explicit about what “sophisticated” is supposed to mean. So maybe even better:

// Since Filelist returns an `os.FileInfo`, this can make it non-ideal for impelmenting Readlink. // This is because the `Name` receiver method defined by that interface defines that it should only return the base name. // However, `Readlink` is required to be capable of returning essentially any arbitrary valid path relative or absolute. // In order to implement this more expressive requirement, implement `ReadlinkFileLister`.

I will just copy youre suggestion here

puellanivis · 2022-10-13T00:37:40Z

request-interfaces.go

+// ReadlinkFileLister is a FileLister that implements the Readlink method.
+// By implementing the Readlink method, it is possible to give a better
+// response than via the default FileLister, since Readlink can return
+// a better result than FileLister (which is limited to os.FileInfo, whose
+// Name() should only return the base name of a file)


I leave this comment block as an exercise for the reader to improve with sembr. I think it’s probably fine and explicitly spelling out what is important.

* add tests for sub directories * add tests for relative symlink targets * improve error messages

ReadlinkFileLister with its Readlink method allows returning paths without misusing the os.FileInfo interface, whose Name() method should only return the base name of a file. By implementing ReadlinkFileLister, it is possible to easily return symlinks of any kind (absolute, relative, multiple directory levels)

georgmu force-pushed the request-server-relative-symlinks branch 2 times, most recently from ac03a58 to 2b5a4f9 Compare September 29, 2022 10:07

puellanivis reviewed Sep 29, 2022

View reviewed changes

georgmu force-pushed the request-server-relative-symlinks branch from 2b5a4f9 to 98d9618 Compare September 30, 2022 15:44

puellanivis reviewed Oct 1, 2022

View reviewed changes

georgmu force-pushed the request-server-relative-symlinks branch from 98d9618 to ec65d7d Compare October 11, 2022 15:42

georgmu force-pushed the request-server-relative-symlinks branch 3 times, most recently from f6172b6 to 40ee72a Compare October 11, 2022 15:53

georgmu force-pushed the request-server-relative-symlinks branch from 40ee72a to 40d3e29 Compare October 12, 2022 11:50

drakkan approved these changes Oct 12, 2022

View reviewed changes

puellanivis reviewed Oct 13, 2022

View reviewed changes

georgmu force-pushed the request-server-relative-symlinks branch from 40d3e29 to 6f60892 Compare October 14, 2022 15:13

georgmu added 3 commits October 14, 2022 17:16

request-server: extend symlink testcase

a3a11ee

* add tests for sub directories * add tests for relative symlink targets * improve error messages

request server: fix symlink handling for relative links

eb2fffb

georgmu force-pushed the request-server-relative-symlinks branch from 6f60892 to 9183e7f Compare October 14, 2022 15:17

drakkan merged commit 43b3a55 into pkg:master Oct 16, 2022

request server: handle relative symlinks #526

request server: handle relative symlinks #526

Conversation

georgmu commented Sep 29, 2022

drakkan commented Sep 29, 2022

georgmu commented Sep 29, 2022

georgmu commented Sep 29, 2022

drakkan commented Sep 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drakkan commented Sep 29, 2022 • edited

drakkan commented Sep 29, 2022

drakkan commented Sep 29, 2022

puellanivis commented Sep 29, 2022

georgmu commented Sep 30, 2022

georgmu commented Sep 30, 2022

drakkan commented Sep 30, 2022 • edited

georgmu commented Sep 30, 2022

drakkan commented Sep 30, 2022

georgmu commented Sep 30, 2022

drakkan commented Sep 30, 2022 • edited

georgmu commented Sep 30, 2022

georgmu commented Sep 30, 2022 • edited

drakkan commented Sep 30, 2022

drakkan commented Sep 30, 2022 • edited

georgmu commented Sep 30, 2022

drakkan commented Sep 30, 2022

drakkan commented Sep 30, 2022

drakkan commented Oct 1, 2022 • edited

puellanivis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgmu Oct 4, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drakkan commented Oct 5, 2022 • edited

drakkan commented Oct 7, 2022

georgmu commented Oct 7, 2022

georgmu commented Oct 11, 2022

drakkan commented Oct 12, 2022

drakkan left a comment

Choose a reason for hiding this comment

puellanivis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drakkan commented Sep 29, 2022 •

edited

drakkan commented Sep 30, 2022 •

edited

drakkan commented Sep 30, 2022 •

edited

georgmu commented Sep 30, 2022 •

edited

drakkan commented Sep 30, 2022 •

edited

drakkan commented Oct 1, 2022 •

edited

georgmu Oct 4, 2022 •

edited

drakkan commented Oct 5, 2022 •

edited