windows username and homedir pathspec not redacted from debuglogs

[five-c-d]

• I have searched open and closed issues for duplicates (but it is hard to find issues about the word 'redacted' rather than issues which happen to mention the word redacted somewhere in the body... mostly I just searched using in:title)

---

Bug description

Debug log obscures/scrubs/redacts some filepaths, but others fail to be obscured.

Steps to reproduce

1. have certain exceptions in your debug log which contain strings of the form file:///C:/Users/NeedsToBeRedactedPlease/AppData/Local/Programs/signal-desktop/resources/app.asar (etc)
2. submit the debug log
3. notice your full legal name (in some cases) appearing in the logfiles

Actual result:

your windows username, or your linux paths or whatever, are uploaded to a world-visible webserver whenever you submit a debuglog

Expected result:

filepath is replaced with file://[REDACTED]/app.asar/ (etc) , which is already the current behavior with [REDACTED]\app.asar\ (etc) with backslashed paths

• old == file:///C:/Users/NeedsToBeRedactedPlease/AppData/Local/Programs/signal-desktop/resources/app.asar (etc)
• new == file://[REDACTED]/app.asar (etc)

Or if the installation-subdirectory matters for some permission-related reason, then consider saying [REDACTED] in most cases and something like [REDACTED / custom subdir] when signal4desktop was installed at a non-default location.

Screenshots

n/a

Platform info

Signal version: Signal/1.17.2 Chrome/61.0.3163.100 Electron/2.0.8 node/8.9.3 env/production

Operating System: Win 10.0 (64-bit)

Linked device version: Android 4.30.x

Link to debug log

Here is a (modified) portion of such a debuglog:

    at Object.getDevices ([REDACTED]\app.asar\ ... 
    at MessageReceiver.onclose (file:///C:/Users/___NeedsToBeRedactedPlease___/AppData/Local/Programs/signal-desktop ... 
    at W3CWebSocket._dispatchEvent [as dispatchEvent] ([REDACTED]\app.asar\ ... 

More info

If you want to kill two birds with one stone while scrubbing file:// URIs as well as backslashed UNC pathspecs, please see also the regex-to-scrub-URLs feature request here, #1939 , which requests something like this alteration to that same privacy.js codebase:

• old == GET https://whispersystems-textsecure-attachments.s3-accelerate.
• old == amazonaws.com/________________654?X-Amz-Algorithm=AWS4-HMAC-SHA256
• old == &X-Amz-Date=201810_______15Z&X-Amz-SignedHeaders=content-type%3Bhost&X-Amz-Expires=3600
• old == &X-Amz-Credential=_________________BDA%2F20181028%2Fus-east-1%2Fs3%2Faws4_request
• old == &X-Amz-Signature=_____________________________________________________________ef9
  
  
• new == GET https://whispersystems-textsecure-attachments.s3-accelerate.
• new == amazonaws.com/[REDACTED]654?X-Amz-Algorithm=AWS4-HMAC-SHA256
• new == &X-Amz-Date=201810[REDACTED]15Z&X-Amz-SignedHeaders=content-type%3Bhost&X-Amz-Expires=3600
• new == &X-Amz-Credential=[REDACTED]BDA%2F20181028%2Fus-east-1%2Fs3%2Faws4_request
• new == &X-Amz-Signature=[REDACTED]ef9

Code which redacts backslashed UNC paths is here, https://github.com/signalapp/Signal-Desktop/blob/master/js/modules/privacy.js (using lots of regex it seems)

[irdan]

It looks like the existing privacy module tests attempted to cover this case: https://github.com/signalapp/Signal-Desktop/blob/development/test/modules/privacy_test.js#L113

If someone wants to assign this bug to me I am happy to work on confirming and resolving this in the near future.

[scottnonnenberg-signal]

@irdan I won't be working on this in the near term, feel free to submit a pull request.

[irdan]

@five-c-d I can't seem to create a test case that reproduces what you are experiencing. Even copying the exact non-redacted strings don't reproduce, though the accuracy of that test is dubious without knowing what your app root is.

The redaction that is performed in the privacy module appears to be very comprehensive. It checks for the app root path in four different formats:

• app root as returned from the path module __dirname
• app root with / substituted for \
• app root with \ substituted for \\
• app root passed through encodeURI

All of those string values are regex escaped and then joined with regex alternation |. That string is turned into a regex with the global flag set to ensure multiple occurrences on the same line are redacted. This is applied to all log text as part of formatting before being passed into the logging system.

Since the strings you shared appear to match the format of a path that would be caught, and it is only sometimes not redacted, I suspect there is something unusual in your environment. Perhaps a symlink or encoding issue is involved.

Is this occurring on a cloned repo or installed from exe? If you can share a more complete debug log as well as the original app root it would help me to reproduce the issue.

[five-c-d]

@irdan , I also peeked at the code, and saw that it seemed to be trying to "do the right thing" but since I didn't see where the problem was I did not claim to have the solution :-) Turns out there might not be a problem with the regex per se, but rather with the detected approot of this particular signal4desktop installation being odd in some fashion.

Is this occurring on a cloned repo or installed from exe?

The system experiencing the trouble is a win10 installation, from the usual signal.org EXE installation method. There is some kind of homedir mirroring being used, for backup-related purposes at the windows-level, details of which are unclear to me.

If you can share a more complete debug log

The debuglog that used to exist was deleted server-side because it contained lastnames, but another can be regenerated if needed. (Though I would prefer to send such a thing via direct email rather than upload it into world-visible github... okay if I email you directly irdan, at the address I found for you online?)

without knowing what your app root is

I've poked around with this somewhat, and am not sure I can actually figure out what the approot is remotely. Short of having my contact with win10 install npm and hand-compile signal4desktop, is there a way to get the approot more easily? They have a running instance of signal4desktop, but is there no incantation they can type into the ctrl+shift+i debugtools thereof which will display the current value of the approot? Or if not that, maybe there is a short and sweet nodeJs incantation that they can pump in from the command-line to extract the approot on their system, without needing to understand too much about the gory details? The person having the trouble is tech-savvy, but not a programmer.

it is only sometimes not redacted

I think that the behavior is not intermittent/unreliable, though I've not gone through a slew of debuglogs from the machine in question. It will reliably scrub out windows-style paths with backslashes like this ==

• at Object.getDevices ([REDACTED]\app.asar\ ...

And it reliably fails to scrub out URL-style paths with forward slashes like this ==

• at MessageReceiver.onclose (file:///C:/Users/FnameLname/AppData/Local/Programs/signal-desktop ...

Like yourself, I now suspect that the approot is somehow "wrong" in the second case, yet "right" in the first case. Let me know if you have further questions, I'll try to get back more promptly, and thank you for looking into this bug, much appreciated

[Ugator]

I can confirm that this bug still exists.
https://debuglogs.org/f0bd968e473f76b3194d4624ccbade799ed2d4aea79cd86c147f6bc4bc3ae823
in this log [REDACTED BY USER] needed to be done by myself (==my Windows logon)
Could we maybe semi-fix this by redacting the windows username seperatly from the root directory. In many cases only that username conatins private information the rest of the path could be standard. Just removing the username also circumvents backslash problematics. We might still need to double check how special chars in usernames might be handled tho.

[five-c-d]

Seconding what Ugator noted, here is a recent debuglog from a problematic system.

Not scrubbed, forward-slashes ==

    at XXXXX (file:///C:/Users/ScrubberFailedToRedactWindows%20Username/AppData/Local/Programs/signal-desktop/resources/app.asar/js/libtextsecure.js:NNNNN:N)
    at file:///C:/Users/ScrubberFailedToRedactWindows%20Username/AppData/Local/Programs/signal-desktop/resources/app.asar/js/libtextsecure.js:NNNNN:NN

Scrubbed properly, backslashes && double-backslashes ==

    at XXXX ([REDACTED]\app.asar\YYY\ZZZ.js:NN:NN)
    at XXXX [as dispatchEvent] ([REDACTED]\app.asar\node_modules\yaeti\lib\EventTarget.js:107:17)
    at [REDACTED]\app.asar\node_modules\node-fetch\index.js:126:13
...Initializing BrowserWindow config: {...,"preload":"[REDACTED]\\app.asar\\preload.js","nativeWindowOpen":true},"icon":"[REDACTED]\\app.asar\\images\\icon_256.png",...}

System running signal4desktop where the above problem was exhibited ==

Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Signal/1.22.0 Chrome/66.0.3359.181 Electron/3.0.14 Safari/537.36 node/10.2.0 env/production
INFO  2019-03-01T08:52:35.001Z app ready
INFO  2019-03-01T08:52:35.019Z updateSchema: Current schema version: 11; Most recent schema version: 11; SQLite version: 3.20.1; SQLCipher version: 3.4.2;
INFO  2019-03-01T08:52:35.025Z Ensure attachments directory exists
INFO  2019-03-01T08:52:35.110Z Initializing BrowserWindow config: {"show":true,"width":1536,"height":824,"minWidth":640,"minHeight":360,"autoHideMenuBar":false,"backgroundColor":"#2090EA","webPreferences":{"nodeIntegration":false,"nodeIntegrationInWorker":false,"preload":"[REDACTED]\\app.asar\\preload.js","nativeWindowOpen":true},"icon":"[REDACTED]\\app.asar\\images\\icon_256.png","maximized":false,"x":0,"y":0}
INFO  2019-03-01T08:52:37.315Z Using OS-level spell check API with locale en_US
INFO  2019-03-01T08:52:37.652Z pre-main prep time: 3 ms
INFO  2019-03-01T08:52:37.678Z Build expires:  2019-05-22T01:30:43.000Z

There is a special character in the windows-username (the SPC character ASCII 0x20 specifically), and the system in question otherwise has an ASCII pathspec although I believe it may be configured with a European-language locale different from en_US ... not sure why the spellcheck of signal4desktop claims it is "using OS-level spell check API with locale en_US" ... maybe that is the electron-framework talking, rather than windows10 talking?