Lnk icon indicating copy to clipboard operation
Lnk copied to clipboard

ArgumentOutOfRangeException when CountCharacters greater than available number of characters

Open securechicken opened this issue 6 months ago • 9 comments

Hi!

When a LNK file contains a StringData structure (such as NAME_STRING, RELATIVE_PATH, WORKING_DIR, COMMAND_LINE_ARGUMENTS and ICON_LOCATION; see MS-SHLLINK as of 9.0 and before, title 2.4) with a CountCharacters value overflowing the actual data that is available in the LNK file, LECmd (as of version 1.5.1.0) fails parsing le LNK file and crashes with a System.ArgumentOutOfRangeException.

This is due to Lnk library (as used by LECmd) not checking the number of bytes that are actually available in the LNK file before attempting to read CountCharacters out of binary content, in LnkFile constructor. As an example, parsing (and reading) of NAME_STRING is done in Lnk/Lnk/LnkFile.cs at line 352:

// Unicode
Name = Encoding.Unicode.GetString(rawBytes, index, nameLen * 2);
...
// ASCII
CodePagesEncodingProvider.Instance.GetEncoding(codepage).GetString(rawBytes, index, nameLen);

As a solution, LnkFile constructor should check that there are enough bytes in rawBytes before attempting to read CountCharacters (*2 for Unicode) out of it.

An attached file is provided so the bug can be reproduced using LECmd as of version 1.5.1.0: LECmd.exe -f ./lnk_6e757794.lnk.

Image

lnk_6e757794.zip

securechicken avatar Jun 17 '25 11:06 securechicken

Kind of irrelevant if the string can't be parsed or are you saying lecmd needs to someone just ignore that string and there's other valid data there?

In other words is that lnk file corrupt or no? Does windows display details properly?

EricZimmerman avatar Jun 17 '25 11:06 EricZimmerman

Yes, I reckon this is confusing... See the other bug #22 which likely explains why this might still be relevant for you: Windows will parse and "run" such a LNK, as well as some other "similar" which will both crash LECmd but perfectly run in Windows.

securechicken avatar Jun 17 '25 11:06 securechicken

length limited to 260.

i would argue windows devs are idiots for not strictly limiting things properly. this is a good example where it just starts randomly doing things when its a wholly invalid string, overly long, etc.

is this better, or worse. who can say? in some ways, having it crash is a more clear indicator that the lnk file is indeed wonky

`Source file: C:\Temp\lnk_6e757794.lnk Source created: 6/17/2025 1:07:41 PM +00:00 Source modified: 6/17/2025 12:34:17 PM +00:00 Source accessed: 6/17/2025 1:20:13 PM +00:00

--- Header --- File size: 3,549,689,113 Flags: HasName, HasArguments, HasIconLocation, IsUnicode, HasExpString, PreferEnvironmentPath File attributes: FileAttributeNormal Icon index: 336 Show window: SwShowminnoactive (Display the window as minimized without activating it.) Target created: 4/14/2034 2:37:26 AM +00:00 Target modified: 9/23/1978 6:54:05 AM +00:00 Target accessed: 11/11/2269 5:37:57 AM +00:00 Name: \u0000 \u0002\u0002 \u0002 \u0002\u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002\u0002 \u0002\u0002\u0002 \u0002 \u0002 \u0002 \u0002\u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002\u0002 \u0002 \u0002 \u0002 \u0002\u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002\u0002\u0002 \u0002 \u0002 \u0002 \u0002\u0002\u0002 \u0002 \u0002\u0002\u0002 \u0002\u0002 \u0002\u0002 \u0002 \u0002 \u0002 \u0002\u0002 \u0002\u0002 \u0002 \u0002 \u0002\u0002 \u0002 Arguments: \u0002 \u0002\u0002 \u0002 \u0002\u0002 \u0002 \u0002\u0002 \u0002 \u0002 \u0002\u0002 \u0002 \u0002\u0002 \u0002\u0002 \u0002 \u0002 \u0002\u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002 \u0002\u0002\u0002 \u0002 \u0002 \u0002\u0002 \u0002 \u0002 \u0002 \u0002 \u0002\u0002\u0002 \u0002 \u0002 \u0002 \u0002\u0002 \u0002\u0002 \u0002 \u0002 \u0002\u0002 \u0002 \u0002\u0002 \u0002 \u0002 \u0002\u0002\u0002 \u0002\u0002\u0002 \u0002\u0002\u0002 \u0002 \u0002 \u0002 \u0002 \u0002\u0002 \u0002\u0002 \u0002\u0002 \u0002 \u0002 Icon Location: c "notepad.exe thisfiledoesnotexistbutitdoesntm`

EricZimmerman avatar Jun 17 '25 13:06 EricZimmerman

Yes I agree this is a very confusing situation, where third party tools have to make arbitrary choices as a result, and users are left with partial information in most cases.

I would maybe do as Windows does (because ultimately, this is where the LNK will be used mostly?), which is limiting all StringData (except command line arguments) to 260 characters maximum. For this example file this will result in properly filled strings. Windows perfectly executes this file, and you can see how it parses it using a COM object in a Powershell term:

$wshell = New-Object -ComObject WScript.Shell
$lnkfile = $wshell.CreateShortcut('lnk_6e757794.lnk')
$lnkfile

securechicken avatar Jun 17 '25 14:06 securechicken

i am less concerned with whether windows parses and executes stuff, and more concerned with showing an examiner what is going on inside it.

windows can just ignore stuff apparently, but i want to make it a useful tool to find this kind of stuff

having LECmd show strange results like that SHOULD result in an examiner diving into the lnk file in a hex editor, etc, so they can see encoded strings etc, to then further dive into anything else thats been attempted to be obfuscated since windows is just idiotic in what it allows

EricZimmerman avatar Jun 17 '25 14:06 EricZimmerman

more ways to pwn these goofy lnk files, bstrings!

Image

Image

EricZimmerman avatar Jun 17 '25 14:06 EricZimmerman

Yes, a simple GNU strings worked all along.

It's really your choice about what to prefer showing or not, how, etc. But in the case you want to help finding out strange stuff first, I figure the most explicit and comprehensive way would be to show both:

  • how it should be parsed according to specification (which for this specific case, would indeed be crashing at some point because a CountCharacters ultimately is wrong per specification... but it would still be interesting to see partial data before);
  • how it is parsed according to Windows maybe just below/next to it. A difference between both should alert, and using only LECmd, will help determining what is the goal on Windows.

securechicken avatar Jun 17 '25 14:06 securechicken

i would certainly add a warning, like

string longer than 260 detected. this is not normal. truncating to 260

or something

EricZimmerman avatar Jun 17 '25 14:06 EricZimmerman

gnu strings is meh. most people do not use it right

EricZimmerman avatar Jun 17 '25 14:06 EricZimmerman