-
Notifications
You must be signed in to change notification settings - Fork 480
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
add MachineKeySessionSecurityTokenHandlerPlugin #172
add MachineKeySessionSecurityTokenHandlerPlugin #172
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 3 changed files in this pull request and generated 2 suggestions.
Files not reviewed (2)
- ysoserial/packages.config: Language not supported
- ysoserial/ysoserial.csproj: Language not supported
Comments skipped due to low confidence (2)
ysoserial/Plugins/MachineKeySessionSecurityTokenHandlerPlugin.cs:24
- The abbreviation 'PoC' should be expanded to 'Proof of Concept'.
* This PoC produces an error and may crash the application
ysoserial/Plugins/MachineKeySessionSecurityTokenHandlerPlugin.cs:93
- [nitpick] The payload string has inconsistent indentation and should be corrected for better readability.
string payload = @"<SecurityContextToken xmlns='http://schemas.xmlsoap.org/ws/2005/02/sc'>
Tip: Leave feedback on Copilot's review comments with the 👎 and 👍 buttons to help improve review quality. Learn more
|
||
byte[] serializedData = (byte[])new TextFormattingRunPropertiesGenerator().GenerateWithNoTest("BinaryFormatter", inputArgs); | ||
DeflateCookieTransform myDeflateCookieTransform = new DeflateCookieTransform(); | ||
MachineKeyDataProtector Protector = new MachineKeyDataProtector(validationKey, decryptionKey, decryptionAlg, validationAlg, purposes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variables validationKey, decryptionKey, decryptionAlg, and validationAlg should be validated to ensure they are not empty or invalid before being used.
MachineKeyDataProtector Protector = new MachineKeyDataProtector(validationKey, decryptionKey, decryptionAlg, validationAlg, purposes); | |
if (string.IsNullOrEmpty(validationKey) || string.IsNullOrEmpty(decryptionKey) || string.IsNullOrEmpty(decryptionAlg) || string.IsNullOrEmpty(validationAlg)) | |
{ | |
throw new ArgumentException("ValidationKey, DecryptionKey, DecryptionAlg, and ValidationAlg must not be empty."); | |
} | |
MachineKeyDataProtector Protector = new MachineKeyDataProtector(validationKey, decryptionKey, decryptionAlg, validationAlg, purposes); |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
/** | ||
* Author: L@2uR1te (@2308652512) | ||
* | ||
* Comments: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The word 'Comments:' should be 'Comment:'.
* Comments: | |
* Comment: |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
I am not sure about some of the libraries inserted in the packages especially as it seems they are from .NET (core). I will have to review it properly thought by downloading and running it to see what we are trying to achieve here and whether we can simplify it. |
Hey @2308652512, I am very sorry for the delay on this. I will have a look at this in the coming week. |
No problem at all! Please feel free to focus on your tasks. I hope this imperfect little plugin won't cause you any inconvenience. :) |
Hi all, just straight out of the box I cannot build https://github.com/2308652512/ysoserial.net/tree/ysoserial.net_addMachinePlugin in VS 2022. I think I need to fix the packages and add the plugin manually to see what's needed there. |
I think the best solution is to merge this inside the I am thinking about what I have done in the I am still not sure how easy it will be but I think this is a better approach. |
@2308652512 I have created a new fork branch to implement what you have in the plugin: https://github.com/irsdl/ysoserial.net/tree/SessionSecurityTokenHandler_Update However, when I use a command to generate an XML for a test website with known Machine Key params, it does not work and it shows the following error:
As you had made this working, could you please have a look to see what's going wrong here? |
I have audited your code, and I believe I have identified the issue. Firstly, the content of the MachineKeyHelper class is nearly identical to the MachineKey class and MachineKeyDataProtector class found in the AspNetTicketBridge dependency. Therefore, it is highly likely that the problem does not lie within the implementation logic of the MachineKeyHelper class, but rather that there are some erroneous practices in the generation of the payload. Consequently, I will attempt to investigate the implementation of the SessionSecurityTokenHandlerPlugin class. The following code segment is relatively straightforward to analyze. When the user enables the -ucmk mode, it corresponds to the original generation logic of the SessionSecurityTokenHandlerPlugin. Here, the Encode() method is called once for encryption:
Then, the encrypted data is Base64 encoded:
Therefore, this part of the logic is sound; the issue lies in the subsequent logic that has been added: When the user enters the following else branch, the following code will be executed:
At this point, the payload generation has actually been completed. However, after the user exits this branch, the following line is executed again: This means that Base64 encoding is effectively performed twice, which clearly results in an invalid payload that cannot be processed correctly. Therefore, I made the following modifications:
The generated payload is now usable, and I believe we have successfully resolved this issue. |
Thanks @2308652512 , nice catch, I have updated the code. Indeed the copy/paste went wrong there. I have also attached the file I used (I like that you also used the name The command I used was:
If you can't see any obvious issue here, then I will need to debug it which may take some time :'( BTW, the attached version.aspx file version.aspx.txt show me |
I believe I have identified the root cause of the issue. The problem lies in the -validationalg="SHA1" parameter provided to ysoserial.exe. I noticed that you modified its default value to HMACSHA256:
I also noticed that the value you provided is actually SHA1, which does not seem to be an expected input. Therefore, when you specify SHA1, it might still default to HMACSHA256? This is inconsistent with the configuration in the web.config file. Consequently, the appropriate approach should be to pass HMACSHA1, specifically as follows:
I have made every effort to replicate your IIS environment, and I have also replaced the web.config with an identical version to yours. Now, I can successfully reproduce the issue. |
Wait a minute, something seems off. I think I may have misanalyzed the . @irsdl Is it possible that MachineKeyHelper supports both SHA1 and HMACSHA1 simultaneously, yielding different results? However, it appears that it indeed expects HMACSHA1 rather than SHA1. If this still doesn't resolve your issue, I can provide details about my environment. |
@2308652512 |
I carefully reviewed the implementation of your a.aspx, and it seems I've identified a difference between our approaches. In fact, we must utilize the following method:
to achieve deserialization. Here, I am providing my a.aspx code for you to try and see if it works successfully. |
@irsdl I believe that the encryption and decryption operations using MachineKey configuration information will only occur when the developer processes data with MachineKeySessionSecurityTokenHandler().ReadToken(). If SessionSecurityTokenHandler().ReadToken() is used, then DPAPI will be employed for encryption and decryption. Of course, my understanding might not be entirely accurate, as I only recently learned about the security issues related to SessionSecurityTokenHandler. :( |
@2308652512 I think you are absolutely right here. This means the other mistake I made was to not get the same test that you had in the code. Last night I had wrongly assumed they are both from the same DLL despite reading your notes. These totally different DLLs and System.IdentityModel.Services.dll as you had mentioned originally. Sorry about my confusion. I need to totally revise this now. |
@pwntester Could you please approve my review comments so @2308652512 can also see them? @2308652512 I have added some comments for the changes to your code so it can be tested and approved. I think you can have it as a separate plugin given it uses a completely different namespace/class for exploitation. Thanks for your patience and sorry again for all the confusions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be able to approve this after these changes are applied. The main issue here are the imports from .NET Core. We don't need any import other than perhaps System.IdentityModel.Services.dll
{"t|test", "In this scenario, the test mode should not be applied, as the sink point relies on the web environment. Default: false", v => test = v != null }, | ||
{"minify", "Whether to minify the payloads where applicable (experimental). Default: false", v => minify = v != null }, | ||
{"ust|usesimpletype", "This is to remove additional info only when minifying and FormatterAssemblyStyle=Simple. Default: true", v => useSimpleType = v != null }, | ||
{"vk|validationKey=", "Enter the validationKey from the web.config", v => validationKey = v }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use lowercase parameters such as validationkey
in the input parameters to remain compatible with the ViewState plugin.
{"minify", "Whether to minify the payloads where applicable (experimental). Default: false", v => minify = v != null }, | ||
{"ust|usesimpletype", "This is to remove additional info only when minifying and FormatterAssemblyStyle=Simple. Default: true", v => useSimpleType = v != null }, | ||
{"vk|validationKey=", "Enter the validationKey from the web.config", v => validationKey = v }, | ||
{"ek|decryptionKey=", "Enter the decryptionKey from the web.config", v => decryptionKey = v }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use lowercase
{"ust|usesimpletype", "This is to remove additional info only when minifying and FormatterAssemblyStyle=Simple. Default: true", v => useSimpleType = v != null }, | ||
{"vk|validationKey=", "Enter the validationKey from the web.config", v => validationKey = v }, | ||
{"ek|decryptionKey=", "Enter the decryptionKey from the web.config", v => decryptionKey = v }, | ||
{"va|validationAlg=", "Enter the validation from the web.config. Default: HMACSHA1. e.g: HMACSHA1/HMACSHA256/HMACSHA384/HMACSHA512", v => validationAlg = v }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use lowercase
{"vk|validationKey=", "Enter the validationKey from the web.config", v => validationKey = v }, | ||
{"ek|decryptionKey=", "Enter the decryptionKey from the web.config", v => decryptionKey = v }, | ||
{"va|validationAlg=", "Enter the validation from the web.config. Default: HMACSHA1. e.g: HMACSHA1/HMACSHA256/HMACSHA384/HMACSHA512", v => validationAlg = v }, | ||
{"da|decryptionAlg=", "Enter the validation from the web.config. Default: AES. e.g: AES/DES/3DES", v => decryptionAlg = v } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use lowercase
using System.IdentityModel.Tokens; | ||
using ysoserial.Helpers; | ||
using System.IdentityModel.Services.Tokens; | ||
using AspNetTicketBridge; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using AspNetTicketBridge
which requires lots of .NET Core dependencies, use https://raw.githubusercontent.com/irsdl/ysoserial.net/refs/heads/SessionSecurityTokenHandler_Update/ysoserial/Helpers/MachineKeyHelper.cs and add it to your project and use it. All imported dependencies should be removed as well (perhaps you want to apply all changes on a clean ysoserial.net code as there are many imports).
Console.WriteLine("Try 'ysoserial -p " + Name() + " --help' for more information."); | ||
System.Environment.Exit(-1); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add some validations to ensure the required parameter such as validationkey are not null or empty.
try | ||
{ | ||
//In this scenario, the test mode should not be applied, as the sink point relies on the web environment. | ||
//Please run the following code in a web environment configured with a MachineKey for experimentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding comments here, consider printing out a message so users will know why -t wouldn't work. Or perhaps remove -t argument altogether and just keep the comments here to show how it can be triggered on the target box.
ysoserial/packages.config
Outdated
@@ -1,15 +1,40 @@ | |||
<?xml version="1.0" encoding="utf-8"?> | |||
<packages> | |||
<package id="AspNetTicketBridge" version="1.6.0" targetFramework="net472" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove all these imported libraries. Just add a reference for System.IdentityModel.Services
ysoserial/ysoserial.csproj
Outdated
@@ -91,6 +91,9 @@ | |||
<Prefer32Bit>true</Prefer32Bit> | |||
</PropertyGroup> | |||
<ItemGroup> | |||
<Reference Include="AspNetTicketBridge, Version=1.6.0.0, Culture=neutral, processorArchitecture=MSIL"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully after just having the System.IdentityModel.Services
as the reference, we will not any changes for any other libraries here.
@irsdl No problem, the discussion above has given me a deeper understanding of this vulnerability. :> |
@irsdl I have modified my code and reset the ysoserial.csproj and packages.config files. It no longer requires AspNetTicketBridge or any .NET Core code. The MachineKeySessionSecurityTokenHandlerPlugin now only depends on MachineKeyHelper to generate the payload. Additionally, I have adjusted the parameter format for the MachineKeySessionSecurityTokenHandlerPlugin as per your suggestions, such as changing validationKey to validationkey, and I have also implemented checks for the validity of these parameters. Finally, I have updated the functionality for the test mode. Now, when users use the -t parameter, they will see a prompt message. I’m not sure if there’s anything I might have overlooked, so please review my code again. |
Thanks, it works for me. I am going to approve this. The only thing needs changing which I can do myself is to replace with |
The Ysoserial.net tool includes an exploit plugin for the SessionSecurityTokenHandler security issue. However, due to the fact that SessionSecurityTokenHandler employs DPAPI for encryption and decryption, it is often difficult to exploit in most cases.
Nevertheless, Microsoft's documentation on SessionSecurityTokenHandler mentions that for web scenarios requiring a similar security mechanism, one can use the MachineKeySessionSecurityTokenHandler. This class inherits from SessionSecurityTokenHandler and shares similar characteristics. The key difference is that MachineKeySessionSecurityTokenHandler utilizes MachineKey configuration information for encryption and decryption operations. Therefore, as long as the MachineKey configuration information can be obtained (for instance, through a web.config leak), it may be possible to exploit it, making it more susceptible to exploitation compared to SessionSecurityTokenHandler.
https://learn.microsoft.com/en-us/dotnet/api/system.identitymodel.services.tokens.machinekeysessionsecuritytokenhandler?view=netframework-4.8.1
You can use code like the one below for testing:
After verifying the feasibility of this issue, I proceeded to generate the payload required for MachineKeySessionSecurityTokenHandler.ReadToken(). The format of this payload is similar to that generated by SessionSecurityTokenHandlerPlugin, with the only difference being in the node section of the XML data. MachineKeySessionSecurityTokenHandler relies on MachineKey configuration information for the encryption and decryption of the Cookie, which typically necessitates a complete web environment. However, I discovered a dependency on https://github.com/dmarlow/AspNetTicketBridge (which can be imported via NuGet) that allows for the generation of Cookies using the MachineKeyDataProtector.Protect() method. This requires the provision of information such as validationKey, decryptionKey, decryptionAlg, validationAlg, and purposes. Consequently, I developed this plugin to generate the payload required for MachineKeySessionSecurityTokenHandler.ReadToken().
I have verified this plugin locally, and it functions correctly. However, it appears that my implementation has made significant changes to packages.config and ysoserial.csproj. There may be better implementation approaches available...