SQLRecon icon indicating copy to clipboard operation
SQLRecon copied to clipboard

Performance issue: CLR DLL upload spends minutes converting bytes to hex

Open chryzsh opened this issue 9 months ago • 2 comments

First, thank you for SQLRecon and its great documentation. We used it on a recent Red Team engagement with great success.

Issue

Uploading even a small (2–3 MB) DLL through the CLR module takes several minutes.

Root cause

_convertDLLToSQLBytesFile and _convertDLLToSQLBytesWeb build the hex string like this:

dllBytes += b.ToString("X2");

(see CLRAssemblies.cs L494).

Because string is immutable, every += allocates a new string and copies all previous content as an O(n²) operation in both time and memory.

Quick‑fix that removes the hot path

This was my first attempt at a quick fix.

byte[] fileBytes = File.ReadAllBytes(dll);
dllBytes = BitConverter.ToString(fileBytes).Replace("-", "");

With this change, our 2–3 MB DLL uploads drop from multiple minutes to near‑instant. It still loads the whole file into memory, which is usually fine and would likely only become a factor for extremely large DLLs.

Alternative quick fixes

Digging more into it, I realized I could probably have used this .NET +5 function (not tested).

dllBytes = Convert.ToHexString(fileBytes);

Similar pre‑.NET 5 solution (also not tested)

var sb = new StringBuilder(fileBytes.Length * 2);
foreach (byte b in fileBytes) sb.Append(b.ToString("X2"));
dllBytes = sb.ToString();

All above solutions avoid the quadratic concatenation, but they still double the payload size by converting to hex.

Possible optimized solution

A more optimized solution would likely be to skip the hex literal entirely and pass the DLL raw bytes via VARBINARY(MAX) parameter (@p_bin) and the assembly name via another parameter (@p_asmName). I would suspect that change is not worth the additional work for typical payload sizes (usually well under 100 MB, e.g., a Sliver payload).

Suggestion

  1. **Merge either of the proposed quick‑fix solutions above (I can open a PR if you prefer).
  2. We can discuss the binary‑parameter upload if you feel like going down that road

Let me know if you would like the PR :)

chryzsh avatar May 04 '25 06:05 chryzsh

Hey @chryzsh - I love getting feedback and success stories other red teams - thanks so much. I appreciate you opening an issue - this makes total sense. I think it makes sense to move forward with the quick fix as it significantly decreases the conversion time and is quite an easy implementation. I’d like to do a little bit more research on the binary approach however.

Regarding updating the code - I’m good with either way - being I make the changes (some time this weekend) or you make a PR. Either way, you’ll absolutely get credit for the work.

Thanks again!

skahwah avatar May 06 '25 21:05 skahwah

Thank you. I think it makes sense for you to pick the solution you consider best aligned and add it yourself. This is totally fine for us, and we are happy for any credit we get.


Another small idea from the side. We used this on the recent engagement to check if SQL auditing is enabled. SELECT SERVERPROPERTY('IsAuditEnabled') AS IsAuditEnabled;", It returns 1/0 for enabled/disabled. Maybe it could be a nice addition to the enum techniques.

chryzsh avatar May 07 '25 06:05 chryzsh

Apologies for the delay on this @chryzsh - work and life got in the way. I'm actively developing and testing out your proposed changes now and will notify you when I push the changes into the 3.10-dev branch. I'll be sure you credit you with the additions. Thanks again!

skahwah avatar Jun 17 '25 14:06 skahwah

Testing and development complete for the DLL optimization issue. You can view a brief video here if you are interested. I'll commit the changes and provide credit in the 3.10-dev branch shortly @chryzsh

skahwah avatar Jun 17 '25 15:06 skahwah

Implemented the AuditStatus module using your query above.

All changes have been committed to the 3.10-dev branch and will be merged into main shortly.

Closing this issue. Thanks for your contributions @chryzsh

skahwah avatar Jun 17 '25 16:06 skahwah

Forgot to ever reply you! Very sorry about that. I just recalled this because I used the CLR module today in a pentest and now I can confirm it works perfectly.

chryzsh avatar Oct 03 '25 17:10 chryzsh

@chryzsh all good! Glad to hear it.

skahwah avatar Oct 03 '25 18:10 skahwah