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
[SR-1957] Remove unsafeAddressOf
#44566
Comments
Comment by Charlie Monroe (JIRA) Maybe I'm missing something, but I use it on several occasions:
Am I missing some initializer on UnsafePointer that takes AnyObject as parameter and returns UnsafePointer<Void>? |
Thanks for the list! > for logging pointer address of the object Unmanaged covers these. > pseudo-random generation - get an unsafe address of something on the stack I'm afraid this is not a valid usecase. Leaking the address of the stack (or the heap) to a 3rd party (a potential attacker) is a security risk because it makes exploiting other vulnerabilities easier by defeating ASLR. |
Comment by Charlie Monroe (JIRA) Ok, I can see how Unmanaged covers this, though it's a bit more verbose:
vs
And I personally dislike that you need to deal with retained/unretained when all you are interested in is the address. Ad the pseudo-random generator - it's not entirely leaking the address of the stack - as I've mentioned, this is used to get some base value, from which we can use e.g. the last few bits (or every second, etc.) which isn't indicative of anything on its own, yet is still enough random for most purposes since the value differs from launch to launch. But without the entire 64 bits, this is IMHO fairly useless to the attacker since the possible stack address range is still incredibly large (when talking about 64-bit architectures), even though it usually is in the higher end of the memory range. |
> And I personally dislike that you need to deal with retained/unretained when all you are interested in is the address. I understand the annotation burden, but it is critical to be explicit in the source what kind of reference kind semantics you want for this cast. For example, when attaching the object to another object through objc_setAssociatedObject, you need to use ".passRetained", not ".passUnretained". > But without the entire 64 bits, this is IMHO fairly useless to the attacker since the possible stack address range is still incredibly large ASLR only randomizes a few bits at the top of the address (not all 64 bits), our CPUs don't even support full 64-bit addresses now, e.g., AMD64 implementations support only 48 bits, and most simple PRNGs are trivially invertible. The risk is real. > yet is still enough random for most purposes since the value differs from launch to launch Why not use a seeding mechanism that was designed for this purpose? For example, reading from /dev/urandom, or using arc4random(). |
Comment by Charlie Monroe (JIRA) Ad objc_setAssociatedObject - for the value of the key, it shouldn't be retained - the run-time (last time I checked) doesn't care about whether the key points to an object or not, all that matters is its actual value which is usually achieved by a reference to something on the heap (usually a string constant): /// ObjC:
static NSString const * NSManagedObjectContextXUSyncManagerKey
= "NSManagedObjectContextXUSyncManagerKey"
objc_setAssociatedObject(obj, &NSManagedObjectContextXUSyncManagerKey,
newValue, OBJC_ASSOCIATION_RETAIN) /// Swift:
private let NSManagedObjectContextXUSyncManagerKey: NSString
= "NSManagedObjectContextXUSyncManagerKey"
objc_setAssociatedObject(self, unsafeAddressOf(NSManagedObjectContextXUSyncManagerKey),
newValue, .OBJC_ASSOCIATION_RETAIN) Reading from /dev/urandom suffers from:
But anyway, that is slightly off topic. I'd personally keep unsafeAddres(of🙂, but move it from a floating global function under Unamanged? extension Unmanaged {
func unsafeAddress(of: T) -> UnsafePointer<Void>
} |
Sure, if you use an object with static lifetime, you don't need to retain it. It is not always the case that people use such objects. In that case, it is critical for code clarity to make the ownership semantics explicit. > Reading from /dev/urandom suffers from: Sorry, I don't see any of these reasons as compelling. If you need random data that's any good, you need to use appropriate APIs. If you don't care, you can use the current time. Not to even mention that the address you get from the stack is not even guaranteed to differ from run to run, ASLR can be off. You can just hardcode a constant of your choice as the seed, it would be as effective. Using current time would be a better choice. (Not a good one, on the global measure of things though; on iOS, arc4random() would be a good choice.) > but the fact that each random number requires a syscall is a bit pricy I was not suggesting to perform I/O for every random number, just for the seed. Also, on iOS, you have arc4random(), which you don't even need to seed yourself or worry about the I/O for the seed. > it is documented that it will only contain something random if the OS has any data to supply If you don't want to wait for the high-quality random data, you can use async I/O and just substitute a 0 as the seed when you hit your timeout. I don't understand why not just always use 0 as the seed though, if low-quality randomness is acceptable. > But anyway, that is slightly off topic. I'd personally keep unsafeAddres(of, but move it from a floating global function under Unamanged? I don't see what issue would this solve. |
Comment by Charlie Monroe (JIRA) Fair points. About the rename/move - it doesn't solve anything, but rather keeps the same functionality categorized rather than "floating around". |
Comment by Charlie Monroe (JIRA) This is now part of apple/swift-evolution#437 |
charlieMonroe (JIRA User) was right about |
Additional Detail from JIRA
md5: 0a2d4f61740d8666fd515693afde6a6b
Issue Description:
We are not aware of any real usecases for `unsafeAddressOf`. If there are any, it should be renamed to `unsafeAddress(of:)` to follow the guidelines.
NOTE: Requires a swift-evolution proposal.
The text was updated successfully, but these errors were encountered: