execute_dotnet_assembly fix parameters managing #14304
Conversation
Fix a problem running assemblies with Main signature (string[] args) and no passed parameters
|
Code changes look good, so I'll move on to testing this. Thanks for catching and fixing this issue. |
|
Quick example showing reproducing the original issue: |
There was a problem hiding this comment.
Alright so I've had a chance to review this more thoroughly and I'm not sure this is the best way to address the issue.
First, while these changes do fix both cases for the .NET binary that takes parameters, now the binary that does not seems to be broken which is a regression. If the binary used a signature that expected arguments, I think it's reasonable that it would fail to run when no parameters are passed.
Next, I'm not a fan of these special argssize numbers for a few reasons. First it's a bit difficult to read and not particularly expressive. Looks like 1 is no arguments, 2 is arguments which may or may not be present? I see that the size includes the NULL terminator for the parameters which is fine, but using 2 as a special value is less than ideal because it's the same when it's double null terminated and when a single character is the argument. It's also difficult from a user perspective to know when datastore['ARGUMENTS'] will be nil and when it will be "" which are handled differently using this logic. I suspect this has something to do with the regression I mentioned.
What I would propose instead is restoring how datastore['ARGUMENTS'] is processed and specify the signature in a separate OptEnum. Something like:
OptEnum.new(['Signature', [true, 'The function signature', 'Automatic', ['Automatic', 'Main()', 'Main(string[])'])
You'd then pass the signature type as another value you like you do argssize and assembly_size. You could easily use 1 for Main() and 2 for Main(string[]). These values should be defined as constants or an enum of some kind. On the Metasploit side you'd do something to the effect of:
params = ""
case datastore['Signature']
when 'Automatic'
signature = datastore['Arguments'].blank? ? 1 : 2
when 'Main()'
signature = 1
when 'Main(string[])'
signature = 2
params << datastore['ARGUMENTS']
end
params << "\x00"
argssize = params.lengthThis would retain the original behavior of assuming no parameters when no arguments are specified but also allow the user to explicitly define it.
Also like in HostingCLR.cpp lines 276 and 278, there's an off by one error. Since raw_args_length includes the NULL terminator, it does not need to be increased by one. You can drop the +1 on each of those two lines.
I know that's alot, so let me konw if you have any questions.
|
Your proposal seemed interesting to me. I had some doubts about the usability, but I implemented and tested it a bit. I found the behavior much more consistent. In addition, having an explicit option for the sign with the For the signature parameter I preferred to use a single byte instead of an integer. |
|
I pushed up some changes in 0ccb50a.
Then with that in place, I tested the parameter and no parameter bins with their correct signatures and the automatic detection. In all cases, they appear to be working as intended. Once the tests pass, I'll go ahead and land this. Thanks! Testing Output |
|
@msjenkins-r7 test this please. |
Release NotesUpdated the |
This PR fix a problem in execute_dotnet_assembly while running assemblies with Main signature (string[] args) and no passed parameters
This module must handle the following main signature cases:
Main(string[] args) (with and without parameters passed)
Main()
The current implementation does not correctly handle the case where the signature is
Main(string[] args)
and no parameters are passed.
Verification
To verify the correct functioning of the fix it is necessary to build the following assembly
TestParameters.exe : source
TestNoParameters.exe : source
Main(string[] args) with no parameter
msfconsoleuse post/windows/manage/execute_dotnet_assemblyset DOTNET_EXE /home/msfusr/builds/TestParameters.exeset SESSION 1runMain(string[] args) with parameters
msfconsoleuse post/windows/manage/execute_dotnet_assemblyset DOTNET_EXE /home/msfusr/builds/TestParameters.exeset ARGUMENTS pipposet SESSION 1runMain()
msfconsoleuse post/windows/manage/execute_dotnet_assemblyset DOTNET_EXE /home/msfusr/builds/TestNoParameters.exeunset ARGUMENTSset SESSION 1run