warden icon indicating copy to clipboard operation
warden copied to clipboard

Modified Regex pattern to accept commands besides full executable path.

Open feldrim opened this issue 6 years ago • 8 comments

Although it's the default behavior, the regex pattern added with commit https://github.com/RainwayApp/warden/commit/4948bd6a31df4ad6c528426302b9ee4e20e9ddce does not accept commands from environment paths, as I have mentioned on issue https://github.com/RainwayApp/warden/issues/6 . So I changed the pattern to accept commands like cmd. Tests are working but they are not ideal solutions, just workarounds. It needs some refactoring yet still capable of esting the condition.

feldrim avatar Dec 26 '18 14:12 feldrim

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Dec 26 '18 14:12 CLAassistant

This will take some time to review due to the holidays. @healingbrew check when you have some time.

andrewmd5 avatar Dec 27 '18 02:12 andrewmd5

The regex seems to return C for C:\Windows\System32\cmd.exe

This is your regex:

"cmd"                                         = cmd
"garbled cmd garbled"                         = garbled
"cmd.exe"                                     = cmd
"garbled cmd.exe garbled"                     = garbled
"C:\cmd.exe"                                  = C
"garbled C:\cmd.exe garbled"                  = garbled
"C:\Windows\system32\cmd.exe"                 = C
"garbled C:\Windows\system32\cmd.exe garbled" = garbled
"\\server\cmd.exe"                            = \
"garbled \\server\cmd.exe garbled"            = garbled
""warden dot dll.exe""                        = "warden
"garbled "warden dot dll.exe" garbled"        = garbled

This is the current implementation

"cmd"                                         = NULL
"garbled cmd garbled"                         = NULL
"cmd.exe"                                     = NULL
"garbled cmd.exe garbled"                     = NULL
"C:\cmd.exe"                                  = C:\cmd.exe
"garbled C:\cmd.exe garbled"                  = C:\cmd.exe
"C:\Windows\system32\cmd.exe"                 = C:\Windows\system32\cmd.exe
"garbled C:\Windows\system32\cmd.exe garbled" = C:\Windows\system32\cmd.exe
"\\server\cmd.exe"                            = \\server\cmd.exe
"garbled \\server\cmd.exe garbled"            = \\server\cmd.exe
""warden dot dll.exe""                        = NULL
"garbled "warden dot dll.exe" garbled"        = NULL

This is our fault for not writing the tests, but this PR cannot be merged until this is fixed. The entire regex is hugely inefficient and complicated. It can be trimmed down to ((([\w])?[:%\\\/]|").*(\.[\w:]+|"|%))|([\w\d.\\:]*(\s|$)) (add path-safe characters like _-()&) But that yields some quirks as well

"cmd"                                         = cmd
"garbled cmd garbled"                         = garbled 
"cmd.exe"                                     = cmd.exe
"garbled cmd.exe garbled"                     = garbled 
"C:\cmd.exe"                                  = C:\cmd.exe
"garbled C:\cmd.exe garbled"                  = garbled 
"C:\Windows\system32\cmd.exe"                 = C:\Windows\system32\cmd.exe
"garbled C:\Windows\system32\cmd.exe garbled" = garbled 
"\\server\cmd.exe"                            = \\server\cmd.exe
"garbled \\server\cmd.exe garbled"            = garbled 
""warden dot dll.exe""                        = "warden dot dll.exe"
"garbled "warden dot dll.exe" garbled"        = garbled

So you can either split it up into two regexes (((([\w])?[:%\\\/]|").*(\.[\w:]+|"|%)) and ([\w\d.\\:]*(\s|$)) as a fallback which is easier to maintain I guess) or figure prioritization out.

yretenai avatar Dec 27 '18 07:12 yretenai

Oh, thanks. I was totally focused on the "cmd" on tutorial. The cases are very detailed and I'll work on unit tests with them. Thank you.

feldrim avatar Dec 27 '18 10:12 feldrim

I actually couldn't get the format except garbled in "garbled "warden dot dll.exe" garbled". We want commands in quotations? Then what's the issue with .exe. Can you please explain the format and expected result?

feldrim avatar Dec 28 '18 08:12 feldrim

Out of those examples, the expected outcome would be

"cmd"                                         = cmd
"garbled cmd garbled"                         = garbled 
"cmd.exe"                                     = cmd.exe
"garbled cmd.exe garbled"                     = cmd.exe 
"C:\cmd.exe"                                  = C:\cmd.exe
"garbled C:\cmd.exe garbled"                  = C:\cmd.exe 
"C:\Windows\system32\cmd.exe"                 = C:\Windows\system32\cmd.exe
"garbled C:\Windows\system32\cmd.exe garbled" = C:\Windows\system32\cmd.exe 
"\\server\cmd.exe"                            = \\server\cmd.exe
"garbled \\server\cmd.exe garbled"            = \\server\cmd.exe 
""warden dot dll.exe""                        = warden dot dll.exe
"garbled "warden dot dll.exe" garbled"        = warden dot dll.exe

yretenai avatar Dec 28 '18 12:12 yretenai

As of now, the tests pass but the part in quotation marks are accepted with any text inside. I believe it should be something like "command command filename.exe" or if it's specific to warden, "warden command filename.exe". Yet, it's incomplete although all tests pass.

feldrim avatar Dec 28 '18 15:12 feldrim

@healingbrew Hi. I have a question. Let's say I typed something like cd "c:\Users\Some User\Desktop\Some Path Containing Spaces". Then the implementation would only care the quoted string. It might be incorrect by design.

zbalkan avatar Jan 15 '19 07:01 zbalkan