12 ms·
Please restore our registers when you’re done with them
- dblohm7 4y agoI used to take point for Mozilla's efforts dealing with third-party interference in our binaries. Browsers are ripe targets for this kind of shit. The stories we could tell...
- harry8 4y agoAnd from our perspective you should! Would be interesting.
- dblohm7 4y agoI did write a little bit! Here's an oldie but a goodie: https://dblohm7.ca/blog/2016/01/11/bugs-from-hell-injected-third-party-code-plus-detours-equals-a-bad-time/ https://dblohm7.ca/blog/2016/01/11/bugs-from-hell-injected-t...
- srcreigh 4y agoPlease write some stuff about this
- dblohm7 4y agoI did write a little bit! Here's an oldie but a goodie: https://dblohm7.ca/blog/2016/01/11/bugs-from-hell-injected-third-party-code-plus-detours-equals-a-bad-time/ https://dblohm7.ca/blog/2016/01/11/bugs-from-hell-injected-t...
- brucedawson 4y agoMcAfee/Trellix says that they have fixed the bug in the latest (7.4.0) version of their disk encryption product. So apparently my guesses about the root cause were correct.
- KingLancelot 4y ago
- bombcar 4y agoIt’s interesting that there is a zero stored in a register and used for hours - is that significantly faster than just using some actual zero each time? Perhaps CPUs need a “always zero” register or some similar menomic to help harden.
- gumby 4y agoMany architectures do have a 0 register because the value is useful. Others have a zero instruction (or both). What would an "actual zero" be -- a literal?
- bombcar 4y agoYeah, something like "mov ax, 0h" but I suppose that is way more memory intensive as you have to load a 0 into memory somewhere and then copy it into the register. It strikes me as somehow the compiler is making assumptions that aren't being enforced by the ... OS? Language? not sure what, but it's assuming functions restore registers used but that isn't enforced by anything. From my (long ago) time there was PUSHA and POPA but I assume those take quite a bit of "oomph" and are avoided if possible.
- gumby 4y ago> It strikes me as somehow the compiler is making assumptions that aren't being enforced by the ... OS? Language? One case of this problem was in a handwritten assembly file. The other was a compiler bug. This is a case where the ABI requires that if you use a certain register you must save its previous value and restore it afterwords; the two independent bugs were cases of forgetting to look after a certain register. An ABI is simply an agreement as to how things should work: what registers you are free to clobber, which you must look after when you use, how certain data must be laid out in memory, etc. ABIs are typically language specific, though there may be a lot of commonality at the very high level (i.e. how you use sections in an ELF file) and low (anybody using unboxed integers probably will do the same thing). You are welcome to violate the ABI as you see fit in your own code. The OS doesn't care; it has its own constraints (how to make a system call, how to pass arguments to each -- though cf above when I talked about ints). So, say, a Lisp compiler can lay out stack frames differently from a C++ compiler because of the languages' different semantics) but if your Lisp program wants to call a library written in C++ it must make sure memory at the call site follows the C++ ABI because that's what the C++ compiler will have assumed.
- mkj 4y agoIt seems that the chrome developers should be able to perform the same binary analysis on the suspect Mcafee software. I guess it's a bit harder without source code to reference side-by-side though.
- brucedawson 4y agoThat would be possible, but we'd have to install the software, then guess which binary was the culprit, and then have some way of finding the function boundaries. My crude analysis technique required on having symbols for chrome.dll to indicate where functions started, so I'd have to have switched tools to something else that could find those.
- saagarjha 4y agoMost disassemblers are fairly decent at guessing function boundaries if the code isn't too bad.
- webstrand 4y agoI wouldn't be surprised if that was a license violation
- pjc50 4y agoOne of the linked pull requests: https://github.com/cisco/openh264/commit/db956674bbdfbaab5acdd3fdb4117c2fef5527e9 https://github.com/cisco/openh264/commit/db956674bbdfbaab5ac... Macros in a macro assembler .. not very nice.
- userbinator 4y agoHe even mentions that it's Windows-specific, both in the commit message and in the article... and then seemingly fails to make it Windows-only? That's "not very nice" either.
- deleted 4y ago[deleted]
- electroly 4y agoThose PUSH_XMM/POP_XMM macros appear to be Windows-only; I think they expand to nothing on other platforms because they contain their own guard for Windows internally. If that's the case, the call sites don't need to guard for it. I'm guessing that obeying this calling convention is the purpose of those macros. https://github.com/cisco/openh264/blob/db956674bbdfbaab5acdd3fdb4117c2fef5527e9/codec/common/x86/asm_inc.asm#L393 https://github.com/cisco/openh264/blob/db956674bbdfbaab5acdd... https://github.com/cisco/openh264/blob/db956674bbdfbaab5acdd3fdb4117c2fef5527e9/codec/common/x86/asm_inc.asm#L433 https://github.com/cisco/openh264/blob/db956674bbdfbaab5acdd...
- brucedawson 4y agoExactly. My understanding of the conventions and macros in those source files is that you declare what registers you will be trashing, and then the registers are saved/restored as required by that platform. On Linux it would be a NOP, and on Windows it saves and restores XMM6 and XMM7 (XMM0-XMM5 are volatile).
- brucedawson 4y agoThe webrtc fix was thematically similar in that the programmer declared what registers were trashed and then the compiler knows which registers need to be saved. I'm not sure why the compiler doesn't notice when registers are used without being declared as being trashed - I'm really not an expert at _writing_ assembly language.
- altilunium 4y ago
- kaapipo 4y agoHow does this add to the conversation?
- namdnay 4y agolooks like someone spamming links to all their personal projects over HN
- dang 4y agoWe ban accounts that post like this, because the community considers it spamming. I'm not going to ban you because you've also posted other things to HN and seem like a legit user. But you've been posting these links much too often, so please stop doing that.
- 323 4y agoSounds like there should be an option when you write assembly code to tell the compiler "please save/restore any register that I'm modifying in this asm code according to the target you are compiling for"
- pjc50 4y agoThat kind of feature is very .. un-assembler. You certainly could, but the assembler doesn't keep track of any of the relevant information, so it would be a larger feature than you expect. It doesn't know the calling convention. It doesn't have a map of registers dirtied by which instructions. It doesn't do reachability analysis, so it doesn't even necessarily know what's "in the function".
- Joker_vD 4y agoDoesn't GCC inline assembly actually track that info, with the "clobber" section?
- masklinn 4y agoYes, but it's not a property of the assembly (or assembler), it's a necessity for the compiler to correctly codegen around the inline assembly. Historically, assemblers have been really dumb, so ABI is not a thing they'd track, especially as... I don't think they know what functions are? So while they can notice call/ret, they have no knowledge of a label being a jump or call target per-se, do they? So you'd need an assembly-like language to encode this sort of information.
- ElevenLathe 4y agoI mean if you have a macro assembler you should be able to write a macro that generates "save registers" instructions before a section of code and "restore registers" instructions after it. The assembler doesn't need to know our care that this section of code is a function. It's interesting how blurry the line is between a good, full-featured assembler and a crappy compiler!
- rocqua 4y agoTo what degree is this possible to check statically? It feels like at least simple breaks of the ABI rules like this can be detected somewhat statically. The author already started with a very simple and incomplete version. In general, I wonder, are there any (many?) static analyzers for assembled binaries.
- fweimer 4y agoCompilers do inter-procedural register allocation and use custom calling conventions for local calls (where “local” can be quite large with LTO), while preserving ABI externally. This means that clobbering a callee-saved register without saving/restoring it in the same function is not necessarily a bug. Curiously, I found a register clobber bug in the NaCl cryptography library today. Apparently, they used a custom assembler-preprocessor (qhasm) that avoids certain classes of bugs and aids with porting, but while the tool seems to actually model the register in some way, it does not treat it as callee-saved.
- brucedawson 4y agoYou could write an arbitrarily complex analyzer to try to find violations but I suspect that the halting problem means that you can never be sure you've found all errors. I think that either crude heuristics (found two bugs!) or UBSan style instrumentation (finds all bugs in code executed under test) is the best set of solutions.
- secondcoming 4y agoUgh, I had a similar issue when I forgot to clobber ecx when using inline assembly for rdtscp. Very annoying.
- codeflo 4y agoWhy does a „3rd party encryption software” appear in the call stack of a user mode process in the first place? Is this another case where a “security” software injects broken DLL files into all processes in your system?
- cpeterso 4y agoYes. That's pretty common, unfortunately.
- JoshTriplett 4y agoI'd love to see Chrome and Firefox and IE working together to detect this kind of thing and put up a malware warning explaining how to uninstall it, and see how quickly it can be eliminated. Seriously, there is zero valid reason to ever inject code into another program, other than as a debugging tool on a system being debugged.
- Wowfunhappy 4y ago> Seriously, there is zero valid reason to ever inject code into another program Sometimes, I use software which does not work the way I would like it to work. When this software is closed source, and the problem sufficiently annoying, I inject code to make it do what I want. (And no, once I do this I don't open bug reports, unless I can reproduce the problem without code injection.) Because that's the great thing about owning a computer, and knowing how to really use it. It's a tool for you to command. Now, will most people ever do this? No. I wish everyone could, but unfortunately, injecting code in a useful way requires a reasonable amount of coding knowledge. But, this is Hacker News. I would hope most users here can come up with lots of valid reasons to inject code into other programs.
- JoshTriplett 4y agoThere's a huge difference between "I, the user of this system, am intentionally injecting code into a process to debug/extend it in a way I want, and if it breaks I'll have a pretty good idea that it might be my injected code's fault and know where to start looking" and "software that I may not even have intentionally installed on this system or understand the function of has broken other software on this system".
- otikik 4y agoNaïve question about ABIs: shouldn’t the caller be responsible for that? If I want a function to restore certain registers, wouldn’t it be simpler if I was the one that save them on my memory, call the function, and then override the registers with whatever values the function set? Otherwise it seems we’re just… asking for trouble, so to speak.
- dannymi 4y agoIt would just be slow for the caller to have to push and pop (say 30) registers in general that the specific callee (and transitive callees) may not even use. Most ABI specify some registers caller-saved, some registers callee-saved (retained unchanged from the perspective of the caller) and some registers scratch (not-saved). In the end it depends on the architecture and on typical workload which are the fastest--and measurements can be made and it can be found out which combination is the fastest on average.
- dzaima 4y agoYou usually wouldn't need to push&pop all registers, just ones that you want to preserve across the call. Regardless, yeah, non-volatile aka callee-saved registers are extremely important for good performance of code that calls functions (esp. loops - without callee-saved registers, you'd have to store the loop counter & length on the stack!)
- dreamcompiler 4y ago...which is exactly what the callee within the loop would have to do if it used those registers. But I guess your point is that in the callee-save case it only happens when it needs to happen, while in the caller-save case it happens every time whether it needs to or not.
- dzaima 4y agoyep, hence why calling conventions usually have both callee-saved and caller-saved registers, so that you only have "unnecessary" stack usage when you need to use more than roughly half of the registers.
- von_lohengramm 4y agoI'm sure some C++ lawyer can correct me, but isn't branching off of `m_ptr` (e.g. `CHECK()`ing the value) after `std::move(m_ptr)` technically Unspecified Behavior since `std::move(m_ptr)` leaves `m_ptr` as an Unspecified Value? It would be up to the compiler to define the behavior if they so pleased, but the C++ spec would not require such behavior to be defined at all.
- tux3 4y agoMoves in C++ don't make the whole object invalid, it just leaves them in a valid but unspecified state, so that at least the destructor can still run. This means calling operator bool on your unique_ptr ought to be fine, because the unique_ptr still has a valid state (you don't know what that state is, it's unspecified, but it's guaranteed to not be radioactive on mere contact. It has to be a valid unspecified state.)
- account42 4y agoMoves in C++ are a function call that can do whatever any other function call can. The type itself can have additional guarantees and the standard library types generally do. In the case of std::unique_ptr the guarantee is explicit that the moved-from unique_ptr is nulled.
- twoodfin 4y agoIt’s an unspecified but required to be valid value for the moved type. The author mentions it’s a smart pointer type, which could easily be defined to act like this.
- CamperBob2 4y agoIt appears that you need to be really smart in order to not blow your own foot off with this "smart pointer." I'll stick to the regular dumb kind, thanks.
- deleted 4y ago[deleted]
- deleted 4y ago[deleted]
- bumholio 4y agoIf one could reliably detect this DLL injection in the process address space, then the correct "fix" is to crash immediately. Authors of such tools should seek another way of accomplishing their goal, preferably one that does not export their own bugs to innocent bystanders.
- dreamcompiler 4y agoIn this particular example, why not just PXOR XMM7 XMM7 before using XMM7 to zero anything? That way the compiler doesn't have to assume that XMM7 is zero, it can know. Yes it's an extra instruction but XORing a register with itself is such a common metaphor for zeroing that register that CPU designers try to make it fast. Edit: Just noticed that Veliladon essentially made the same comment herein and explained the reason why it's not done this way.
- puffoflogic 4y agoIt's not just that. If you can't trust the ABI then everything else is wrong too. Everything. Not just zeroing registers. It only just so happens that in this case XMM7 is used for zeroing, but it could be used to save a variable across a function call. Then there is no trick to get it set back to the right value. The ABI is not optional, or best effort, or best practice, or any other BS that passes in the ordinary world. It is just as required as the correct operation of instructions (e.g., add should actually add things, mul should actually multiply them, and so on).
- mrkeen 4y agoMy reading of it was that the bug was caused by XMM7 not being restored to its previous value. E.g. on Linux, functions should restore the values of ebx, esi, edi, ... once they're done with them. The article says (on Windows) that XMM7 needs to be restored too. If you can't trust one register being preserved (per the ABI), then you really can't trust the values of any registers.
- dreamcompiler 4y agoFair point, which is why I used the phrase "this particular example." In fact the compiler's lazy assumption about XMM7 was the key to determining that the ABI was being violated. It would have taken longer to figure this out if the compiler had been doing things my way, because then a failure of the callee to preserve XMM7 wouldn't have mattered.
- mjg59 4y ago
- pifm_guy 4y agoI think the real fix is to set all registers to a canary value at the start of int main(), and then when exiting check the canary value is still there. If anything has messed with any registers without permission, you crash and collect as much data as possible about any injected dll's. Then you correlate these to find the culprits, and for each you contact the authors of the DLL and figure out a way to block the injection of any unfixed versions that cause crashes.
- TillE 4y agoCode can be injected anywhere. You'd have to add checks to every single function.
- andreareina 4y agoAny good function upstream of the bad one that also uses the same register will mask the issue
- ip26 4y agoEach runtime function call would probably need its own unique value, perhaps stashed to memory.