On Thu, Jul 16, 2020, at 02:36, Stephen J. Turnbull wrote:
I was asking for the current Unpickler class, which currently has a whitelist hook for loading globals,
Callables are globals in this sense.
not all callables are globals, as has been pointed out attributes of objects (methods) can also be callable.
this does require calling getattr which is itself global, but you can't exercise any fine-grained control
OK. Conceded. Since you have control over load_reduce (if you use the pure Python version), you can parse the stack and get the argument to getattr, but that's a hill I am quite unwilling to die on. That kind of messing around should indeed be enabled by the pickle API with well-defined semantics -- if the use cases justify it.
I think I got all of them, but if you think there may be others feel free to be an extra pair of eyes. But these overrides are not available for the C version,
That's going to be a sticking point, as many pickle use cases want to be as fast as possible. Additional overhead is likely to be unwelcome, although I guess the default would be minimal (I guess checking for the default of None and only calling if non-None would be fastest and do the job).
The idea that the pickle format is "inherently unsafe" and cannot be made safe is magical thinking.
I think you're quite wrong. Pickle format itself is inherently unsafe because it allows a pickle to specify code to be executed on data that pickle specifies.
Which is why an unpickler that *does not directly execute the specified code* is necessary and sufficient to be safe.
It certainly is *not* sufficient to be safe, if the threat model includes, say, a zero-day in the code being called, or an extended attack in which one allowed call sets the stage for another allowed call to blow up. And it may be sufficient to be useless, depending on the use case. You *are* going to die on *that* hill, you know.
On a mostly unrelated note I also have to admit I am baffled why the NEWOBJ opcodes are defined to call __new__ instead of __newobj__, when the latter is expected to exist and be a valid reduce function.
A lot of these decisions have implications for backward compatibility of pickles. If you add code to check versions and decide whether to call __new__ or __newobj__, that has performance and complexity implications that may have been judged not worth the marginal improvement in security. Again, a request to change this seems likely to get pushback.
In any case, to get fairly definitive answers to your questions, you should try writing Antoine and/or Tim off-list.
Footnotes:  I do believe the use cases you have in mind are marginal and the kind of code you'll need to write to take advantage of the features vulnerability-prone. I don't matter, but I suspect the core devs will feel that way too.