Coding Horrors: You can gather all your technical debt into one easy framework!

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »

lifg: Dec 4, 2000; <this tag left blank>; Muldoon

What the hell are y'all doing that you're hitting browser compatibility problems? Modern Javascript frameworks?

I can get near-pixel-perfect precision across browsers with just last year's HTML/CSS and a polyfill.

# ? Mar 26, 2017 04:35

Adbot: ADBOT LOVES YOU

# ? May 16, 2024 16:16

vOv: Feb 8, 2014

RandomBlue posted:

The only browser that I've seen having compatibility issues are still IE and Edge, more Edge than IE10/11 now. You almost have to go out of your way to write stuff that doesn't work on Chrome, FF and Safari now.

Here's a fun .NET issues I ran into recently, find the WTF?, it's pretty easy to spot. Not sure if this is just another .Net Core issue or if older framworks had this problem too.

Path.Combine("C:\Some\SafeDir\", "test.txt") == C:\Some\SafeDir\test.txt
Path.Combine("C:\Some\SafeDir\", "anotherDir", "test.txt") == C:\Some\SafeDir\anotherDir\test.txt
Path.Combine("C:\Some\SafeDir\", "C:\Windows") == C:\Windows
Path.Combine("C:\Some\SafeDir\", "/Windows") == C:\Windows

Thanks assholes. The documentation even says if param2 starts with a root path param2 is what is returned, peroid. Someone actually made the design choice that this behavior was good and perfectly fine.

Looks like it's actually been this way a long long time, since .Net 1.1: https://msdn.microsoft.com/en-us/library/fyy7a5kt(v=vs.71).aspx

Python's os.path.join works the same way. What would you expect it to do?

# ? Mar 26, 2017 04:42

pokeyman: Nov 26, 2006; That elephant ate my entire platoon.

RandomBlue posted:

The only browser that I've seen having compatibility issues are still IE and Edge, more Edge than IE10/11 now. You almost have to go out of your way to write stuff that doesn't work on Chrome, FF and Safari now.

Here's a fun .NET issues I ran into recently, find the WTF?, it's pretty easy to spot. Not sure if this is just another .Net Core issue or if older framworks had this problem too.

Path.Combine("C:\Some\SafeDir\", "test.txt") == C:\Some\SafeDir\test.txt
Path.Combine("C:\Some\SafeDir\", "anotherDir", "test.txt") == C:\Some\SafeDir\anotherDir\test.txt
Path.Combine("C:\Some\SafeDir\", "C:\Windows") == C:\Windows
Path.Combine("C:\Some\SafeDir\", "/Windows") == C:\Windows

Thanks assholes. The documentation even says if param2 starts with a root path param2 is what is returned, peroid. Someone actually made the design choice that this behavior was good and perfectly fine.

Looks like it's actually been this way a long long time, since .Net 1.1: https://msdn.microsoft.com/en-us/library/fyy7a5kt(v=vs.71).aspx

I can't find it.

# ? Mar 26, 2017 05:05

Sinestro: Oct 31, 2010; The perfect day needs the perfect set of wheels.

I typically prefer my invalid function arguments to give errors, personally.

Edit:

RandomBlue posted:

The only browser that I've seen having compatibility issues are still IE and Edge, more Edge than IE10/11 now. You almost have to go out of your way to write stuff that doesn't work on Chrome, FF and Safari now.

Outside of WebRTC on Safari... I really can't believe that's a problem.

# ? Mar 26, 2017 05:06

Meat Beat Agent: Aug 5, 2007; felonious assault with a sproinging boner

RandomBlue posted:

Looks like it's actually been this way a long long time, since .Net 1.1: https://msdn.microsoft.com/en-us/library/fyy7a5kt(v=vs.71).aspx

A lot longer than that, in a way (what does "cd \Windows" do?)

# ? Mar 26, 2017 05:13

RandomBlue: Dec 30, 2012; hay guys!; Biscuit Hider

vOv posted:

Python's os.path.join works the same way. What would you expect it to do?

Look at those last two examples, Path.Combine() is just discarding param1 and only returning param2. What I'd expect it to do in a Windows environment is throw an exception when you try and combine two absolute paths ("C:\SomeDir\" and "C:\Windows\") and not just give me the second option. The other issue is that for every OTHER case with Path.Combine() under windows it converts "/" to "\" and treats it like a normal path separator. Instead when you call Path.Combine("C:\SomePath\", "/SensitivePath") it returns "C:\SensitivePath" when it should either throw an exception or return "C:\SomePath\SensitivePath".

e: To further clarify my last sentence there, Path.Combine("C:\Path1", "Path2/Path3") returns "C:\Path1\Path2\Path3", which is fine.

vOv posted:

Python's os.path.join works the same way. What would you expect it to do?

See above.

Sinestro posted:

I typically prefer my invalid function arguments to give errors, personally.

Exactly, I'm amazed there are this many posters that either have no problem with the behavior I described or don't see the issue.

Meat Beat Agent posted:

A lot longer than that, in a way (what does "cd \Windows" do?)

That's completely different. To compare apples to apples you'd have to ask "What does 'cd \Windows \Windows' or 'cd C:\WindoiwsC:\windows' do?" and the answer is throw an error, which is the right way to handle that.

There is no valid way to combine the paths "C:\Somewhere" and "C:\SomewhereElse". Path.Combine()'s implementation is like saying Apple + Orange == Orange.

RandomBlue fucked around with this message at 05:36 on Mar 26, 2017

# ? Mar 26, 2017 05:32

ShoulderDaemon: Oct 9, 2003; support goon fund; Taco Defender

RandomBlue posted:

Exactly, I'm amazed there are this many posters that either have no problem with the behavior I described or don't see the issue.

The design case for this function (and seriously, almost every language has this function, it's a very common use case) is along the lines of "normalize a possibly relative path provided by a trusted source". You are misusing the function to try to restrict the paths available to an untrusted source, which is almost the exact opposite of what the function is for. You should probably instead be filtering the input to ensure it doesn't have something like ".." and doing other sanitization on the input.

# ? Mar 26, 2017 05:50

Harik: Sep 9, 2001; From the hard streets of Moscow
First dog to touch the stars; Plaster Town Cop

Dylan16807 posted:

Pinning to hash should be just as secure as a signature, albeit less flexible. Signatures don't get applied to entire files; they hash internally.

Upgrading the hash is a separate issue.

Unless it doesn't verify the hash or something?

Harik posted:

You can pin by shasum but no guarantee that the hash that NPM came up with is the same as the file uploaded by the author

To expand on that - just because it's the same file you got before doesn't mean it was the file that was intended to be public. You have to trust NPM.com or whatever, because they're the ones that "sign" (in a loose sense - fetching via https what should be their official word on the shasum, not an actual crypto signature) the hash.

Basically it only protects from someone going in to NPM and dumping random backdoors on everything with a one-shot access. If someone gets persistent access they can make subtle changes to large packages transparently, and those changed versions get pinned by people, etc.

Oh and it only protects if you pin via SHA instead of via major,minor,wildcard patchlevel. You know webdevs don't generally bother with that, just get the latest patches every time their instance spins up!

uncurable mlady posted:

the real coding horrors are the people who use npm without a proxy/replicated feed

I think that's rather a hackaround for something that should be more fundamental - when I download a third party library in basically anything else, I can make deployable artifacts from it. The cycle is dev download, compile (or minify or optimize or whatever is approprate for the language), release artifacts. Deployment uses only those released artifacts. If the original goes poof on the github or whatnot, you are only hosed when it comes to future development and you didn't keep internal copies. NPM (and PIP, for that matter) have a download-is-deploy mentality that you have to go to some lengths to sanitize. I don't think a proxy actually fixes things - you shouldn't be doing an install cycle on every instance spinup.

E: This is probably spiraling into devops horrors/what the gently caress is wrong with webdev/secfuck

Harik fucked around with this message at 06:24 on Mar 26, 2017

# ? Mar 26, 2017 05:54

Vanadium: Jan 8, 2005

yo that's just how combining paths is defined. It's useful. Like you have a base path and then the user gives you another path and you can just loving combine them instead of writing down your own case analysis about the possible combinations of relative and absolute and relative-but-with-a-drive-letter paths or whatever.

# ? Mar 26, 2017 07:21

RandomBlue: Dec 30, 2012; hay guys!; Biscuit Hider

Vanadium posted:

yo that's just how combining paths is defined. It's useful. Like you have a base path and then the user gives you another path and you can just loving combine them instead of writing down your own case analysis about the possible combinations of relative and absolute and relative-but-with-a-drive-letter paths or whatever.

But yo, instead everyone gets to loving think of all the possible ways that can be exploited and write their own input sanitation routines because why default to a secure way of handling it when you can just silently "fix" it and return a result. But dawg, directory/path traversal exploits don't even happen.

RandomBlue fucked around with this message at 07:47 on Mar 26, 2017

# ? Mar 26, 2017 07:39

rjmccall: Sep 7, 2007; no worries friend; Fun Shoe

RandomBlue posted:

But yo, instead everyone gets to loving think of all the possible ways that can be exploited and write their own input sanitation routines because why default to a secure way of handling it when you can just silently "fix" it and return a result. But dawg, directory/path traversal exploits don't even happen.

The function you're looking for � "I want to treat this path as the effective root directory for this access path, which should not be able to escape it" � is indeed theoretically useful. The problem is that it's not actually possible to do that as an abstract path manipulation without making assumptions about the contents of that directory structure, because of the existence of upward links in the directory structure, including ".." but not limited to it. There is a reason why the security consensus is that you should use your operating system's actual sandboxing capabilities, e.g. SetThreadToken on Windows, instead of emulating them with hacked-up path manipulation.

# ? Mar 26, 2017 08:43

nielsm: Jun 1, 2009

RandomBlue posted:

But yo, instead everyone gets to loving think of all the possible ways that can be exploited and write their own input sanitation routines because why default to a secure way of handling it when you can just silently "fix" it and return a result. But dawg, directory/path traversal exploits don't even happen.

As others have already mentioned, the purpose of the function is to resolve a possibly relative path2 given a known absolute path1. It's not to sanitize input. It never claimed to be suited for sanitizing input.

I'm not sure how this documentation for the function can be unclear in any way:

quote:

Return Value
A string containing the combined paths. If one of the specified paths is a zero-length string, this method returns the other path. If path2 contains an absolute path, this method returns path2.

Maybe read the documentation before blindly calling library functions.

# ? Mar 26, 2017 08:44

RandomBlue: Dec 30, 2012; hay guys!; Biscuit Hider

rjmccall posted:

The function you're looking for � "I want to treat this path as the effective root directory for this access path, which should not be able to escape it" � is indeed theoretically useful. The problem is that it's not actually possible to do that as an abstract path manipulation without making assumptions about the contents of that directory structure, because of the existence of upward links in the directory structure, including ".." but not limited to it. There is a reason why the security consensus is that you should use your operating system's actual sandboxing capabilities, e.g. SetThreadToken on Windows, instead of emulating them with hacked-up path manipulation.

First, that's not what I was looking for, I was primarily using it as a way to combine paths using cross-platform path separators. Path.Combine() seemed the way to go at first glance but my testing showed it wasn't really what I wanted. If you search for ".net safely combine directory paths" the first couple of pages point to Path.Combine() with only one or two related mentions in SE results discussing this issue. Sandboxing an app solely to prevent path traversal exploits in something as simple as combining a base and relative paths into a single path seems like overkill. The problem isn't that what I'm trying to do is hard, the problem is the method is named "Combine" when what it is actually doing is more resolution or step by step "CD" simulation than the commonly understood definition of "combine".

nielsm posted:

As others have already mentioned, the purpose of the function is to resolve a possibly relative path2 given a known absolute path1. It's not to sanitize input. It never claimed to be suited for sanitizing input.

I'm not sure how this documentation for the function can be unclear in any way:

Maybe read the documentation before blindly calling library functions.

I never claimed the method was supposed to sanitize input, though sanitizing input by default and not "combining" uncombinable absolute paths would be a safer way to go. There's a reason why almost every SQL library does sanitation on input parameters now unless you explicitly execute SQL statements as strings. The documentation summary (and autocomplete tooltip) says this:

Combines two strings into a path.

The only reason most developers ever look up the documentation on a method is when things aren't working as expected or they have absolutely no idea how to do what they're trying to do. Regardless, I stated from my first post about this issue that the method is working as documented, but IMO the choice to behave this way was a poor one, especially when security is becoming more and more of a concern.

Maybe read my posts before telling me to read things I already stated I read and understood.

# ? Mar 26, 2017 09:21

DaTroof: Nov 16, 2000; CC LIMERICK CONTEST GRAND CHAMPION
There once was a poster named Troof
Who was getting quite long in the toof

It totally makes sense to me that Path.Combine behaves that way when the second parameter is a root path. If it behaved otherwise, I'd probably wind up writing a wrapper function that fit my typical use case.

# ? Mar 26, 2017 09:32

Mr Shiny Pants: Nov 12, 2012

This seems to be the root cause: http://stackoverflow.com/questions/53102/why-does-path-combine-not-properly-concatenate-filenames-that-start-with-path-di

Which is dumb IMO.

Mr Shiny Pants fucked around with this message at 09:40 on Mar 26, 2017

# ? Mar 26, 2017 09:33

NihilCredo: Jun 6, 2011; iram omni possibili modo preme:
plus una illa te diffamabit, quam multæ virtutes commendabunt

The most grokkable explanation seems to be that it mimics the behaviour of calling "cd" on each argument in sequence.

# ? Mar 26, 2017 10:12

Mega Comrade: Apr 22, 2004; Listen buddy, we all got problems!

lifg posted:

What the hell are y'all doing that you're hitting browser compatibility problems? Modern Javascript frameworks?

I can get near-pixel-perfect precision across browsers with just last year's HTML/CSS and a polyfill.

An example we had the other week at my work was displaying PDFs from base64. IE and edge will automatically attempt to download it instead of displaying it and we had to hack around that issue.

When ever we have an issue of a browser not working quite right it's always IE/Edge.

# ? Mar 26, 2017 11:50

Bruegels Fuckbooks: Sep 14, 2004; Now, listen - I know the two of you are very different from each other in a lot of ways, but you have to understand that as far as Grandpa's concerned, you're both pieces of shit! Yeah. I can prove it mathematically.

Mega Comrade posted:

An example we had the other week at my work was displaying PDFs from base64. IE and edge will automatically attempt to download it instead of displaying it and we had to hack around that issue.

When ever we have an issue of a browser not working quite right it's always IE/Edge.

I'd tell you to use blob instead of datauri in this use case, but it wouldn't work if you put the blob in an iframe in IE!

# ? Mar 26, 2017 13:56

CPColin: Sep 9, 2003; Big ol' smile.

NihilCredo posted:

The most grokkable explanation seems to be that it mimics the behaviour of calling "cd" on each argument in sequence.

Yeah, it's not "path1 + path2," it's more like "path1 and-then path2," looks like. I can see how one could assume normal concatenation logic would apply, though.

# ? Mar 26, 2017 16:08

NihilCredo: Jun 6, 2011; iram omni possibili modo preme:
plus una illa te diffamabit, quam multæ virtutes commendabunt

If you want actual concatenation, you can just use String.Join('/'). Forward slashes work on both Windows and *nixes, and both treat any sequence of consecutive slashes in the middle of a path as if they were a single one, so C:/Folder///Subfolder is the same as C:/Folder/Subfolder.

# ? Mar 26, 2017 17:23

rjmccall: Sep 7, 2007; no worries friend; Fun Shoe

RandomBlue posted:

First, that's not what I was looking for, I was primarily using it as a way to combine paths using cross-platform path separators. Path.Combine() seemed the way to go at first glance but my testing showed it wasn't really what I wanted. If you search for ".net safely combine directory paths" the first couple of pages point to Path.Combine() with only one or two related mentions in SE results discussing this issue. Sandboxing an app solely to prevent path traversal exploits in something as simple as combining a base and relative paths into a single path seems like overkill.

If you don't validate the relative path, simple path concatenation will also leave you vulnerable. If you're just trying to be forgiving of trusted but error-prone users, that's one thing, but then why keep talking about exploits and security, and are you really sure you shouldn't trust them and use an absolute path if they give you one? But if this string is actually untrusted, and you're too smart and pretty to just drop privileges for a second, you need to be validating it in ways that should define away any potential chicanery, like validating that it's always (trusted directory name) + (path separator) + (acceptable filename).

# ? Mar 26, 2017 17:29

FamDav: Mar 29, 2008

RandomBlue posted:

Sandboxing an app solely to prevent path traversal exploits in something as simple as combining a base and relative paths into a single path seems like overkill.

Always sandbox, as much as you can.

# ? Mar 26, 2017 18:17

vOv: Feb 8, 2014

Even if you don't want to sandbox, the safe approach is to join the two paths, call whatever function your language provides to 'normalize' a path by resolving all symlinks and ..s, and verify that it's a descendant of your original root.

# ? Mar 26, 2017 18:21

Mega Comrade: Apr 22, 2004; Listen buddy, we all got problems!

Bruegels Fuckbooks posted:

I'd tell you to use blob instead of datauri in this use case, but it wouldn't work if you put the blob in an iframe in IE!

We did, still had problems.

# ? Mar 26, 2017 20:05

necrobobsledder: Mar 21, 2005; Lay down your soul to the gods rock 'n roll; Nap Ghost

All this talk of file paths make me want to try writing programs exclusively with inodes for everything on my local filesystems and simply hash user-provided filenames into UUIDs after canonicalizing them.

# ? Mar 27, 2017 00:36

zergstain: Dec 15, 2005

The behaviour is the same whether the second argument is "/Windows" or "\Windows", right?

# ? Mar 27, 2017 00:54

Vanadium: Jan 8, 2005

necrobobsledder posted:

All this talk of file paths make me want to try writing programs exclusively with inodes for everything on my local filesystems and simply hash user-provided filenames into UUIDs after canonicalizing them.

The only file you should ever be opening is your app's sqlite db anyway. :smug:

When I tried escaping stringly-typed path manipulation hell by deferring to boost::filesystem, I only ran into a crash due to an exception safety bug in their directory iterator, thanks a lot C++ error handling.

# ? Mar 27, 2017 01:05

Hughlander: May 11, 2005

necrobobsledder posted:

All this talk of file paths make me want to try writing programs exclusively with inodes for everything on my local filesystems and simply hash user-provided filenames into UUIDs after canonicalizing them.

Laugh all you will but 20 years ago I got a huge performance boost writing a kernel driver to recognize indie:377373 as a file name meant open up that I ode and go to town. It was 30-40x speed up.

# ? Mar 27, 2017 04:22

Dirty Frank: Jul 8, 2004

vOv posted:

Even if you don't want to sandbox, the safe approach is to join the two paths, call whatever function your language provides to 'normalize' a path by resolving all symlinks and ..s, and verify that it's a descendant of your original root.

This is what we do and I often wonder if its a good idea or not, is it the accepted solution?

# ? Mar 27, 2017 08:18

Malcolm XML: Aug 8, 2009; I always knew it would end like ｔｈｉｓ．

Maybe in 20 years well get pledge() as a universal syscall and I can sandbox to.my hearts delight

# ? Mar 27, 2017 10:19

SupSuper: Apr 8, 2009; At the Heart of the city is an Alien horror, so vile and so powerful that not even death can claim it.

zergstain posted:

The behaviour is the same whether the second argument is "/Windows" or "\Windows", right?

Yes, Windows' posix compatibility means \ and / are largely interchangeable and root paths without a drive letter are just assumed to be the system drive, rather than a subfolder.

# ? Mar 27, 2017 12:21

Munkeymon: Aug 14, 2003; Motherfucker's got an
armor-piercing crowbar! Rigoddamndicu𝜆ous.

Dirty Frank posted:

This is what we do and I often wonder if its a good idea or not, is it the accepted solution?

Yeah

Edit for content: always turn user entered paths into canonical paths with whatever your OS provides before doing anything with them. Well, other than combining them, I guess, but until you get a canonical path from the OS it still has user stink on it

Munkeymon fucked around with this message at 20:55 on Mar 27, 2017

# ? Mar 27, 2017 17:43

zergstain: Dec 15, 2005

SupSuper posted:

Yes, Windows' posix compatibility means \ and / are largely interchangeable and root paths without a drive letter are just assumed to be the system drive, rather than a subfolder.

I asked since it looked like RandomBlue was complaining about inconsistent behaviour with path separators. At any rate, I can see the rationale for why Path.Combine() works that way.

I don't really do .NET, but it looks like this answer I found on SO would work for making sure given two paths, that one is a subdirectory of the other. Or just reject any absolute paths or paths with '.' or '..'

# ? Mar 27, 2017 21:14

Jaded Burnout: Jul 10, 2004

Consistent behaviour is for chumps.

code:

irb(main):001:0> File.join("/a/b", "/c/d")
=> "/a/b/c/d"
irb(main):002:0> Pathname.new("/a/b") + Pathname.new("/c/d")
=> #<Pathname:/c/d>

# ? Mar 27, 2017 22:07

FrantzX: Jan 28, 2007

zergstain posted:

I asked since it looked like RandomBlue was complaining about inconsistent behaviour with path separators. At any rate, I can see the rationale for why Path.Combine() works that way.

I don't really do .NET, but it looks like this answer I found on SO would work for making sure given two paths, that one is a subdirectory of the other. Or just reject any absolute paths or paths with '.' or '..'

What about paths with ".." in the middle, for example: c:\Windows\System32\..\..\Windows\..\ProgramData
Try typing that into Explorer. It works.

# ? Mar 28, 2017 03:14

Meat Beat Agent: Aug 5, 2007; felonious assault with a sproinging boner

Those were already mentioned in the post.

# ? Mar 28, 2017 03:16

RandomBlue: Dec 30, 2012; hay guys!; Biscuit Hider

zergstain posted:

I asked since it looked like RandomBlue was complaining about inconsistent behaviour with path separators. At any rate, I can see the rationale for why Path.Combine() works that way.

I don't really do .NET, but it looks like this answer I found on SO would work for making sure given two paths, that one is a subdirectory of the other. Or just reject any absolute paths or paths with '.' or '..'

I would like to point out that Path.Combine() works exactly like string concatenation using standard path separators EXCEPT in the case where multiple absolute paths are given.

So Path.Combine("C:\Test\Test2","..\..") returns "C:\Test\Test2\..\.." instead of "C:\". Path.GetFullPath() actually resolves the path like CD does and the only time Path.Combine() actually works like CD is when multiple absolute paths are used, which further proves my point that it's poorly named and has confusing behavior. But at this point it's less a coding horror and more of a never-ending argument apparently so it probably belongs in the .Net thread instead of here.

# ? Mar 28, 2017 09:06

Absurd Alhazred: Mar 27, 2010; by Athanatos

C++ code:

auto error = complicatedAPICall(data);
if (error == ERROR_TYPE_I_AM_TRYING_TO_HANDLE)
{
  TYPE dataFixed = handleThisErrorTake9(data);
  auto error = complicatedAPICall(dataFixed);
}
assert(error == NO_ERROR);

Any of you want to guess why I kept triggering the assert?

# ? Mar 29, 2017 01:40

ulmont: Sep 15, 2010; IF I EVER MISS VOTING IN AN ELECTION (EVEN AMERICAN IDOL) ,OR HAVE UNPAID PARKING TICKETS, PLEASE TAKE AWAY MY FRANCHISE

Absurd Alhazred posted:

C++ code:

auto error = complicatedAPICall(data);
if (error == ERROR_TYPE_I_AM_TRYING_TO_HANDLE)
{
  TYPE dataFixed = handleThisErrorTake9(data);
  auto error = complicatedAPICall(dataFixed);
}
assert(error == NO_ERROR);

Any of you want to guess why I kept triggering the assert?

Does the "auto error" inside the if lose scope at the end of the if block, leaving you with the previous error that we already know is bad?

# ? Mar 29, 2017 01:44

Adbot: ADBOT LOVES YOU

# ? May 16, 2024 16:16

Absurd Alhazred: Mar 27, 2010; by Athanatos

ulmont posted:

Does the "auto error" inside the if lose scope at the end of the if block, leaving you with the previous error that we already know is bad?

Yyyyep!

# ? Mar 29, 2017 01:54

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »