Pickle `NEWOBJ`/`NEWOBJ_EX` opcodes don't type check `arg`

[Legoclones]

Bug report

Bug description:

Both the NEWOBJ and NEWOBJ_EX opcodes build a new class instance using user-provided arguments (arg). Both the pickletools code comments and the C accelerator _pickle module enforce the type of arg to be a tuple, but Python pickle does not.

cpython/Lib/pickle.py

Lines 1624 to 1636 in 4c15505

	def load_newobj(self):
	args = self.stack.pop()
	cls = self.stack.pop()
	obj = cls.__new__(cls, *args)
	self.append(obj)
	dispatch[NEWOBJ[0]] = load_newobj
	def load_newobj_ex(self):
	kwargs = self.stack.pop()
	args = self.stack.pop()
	cls = self.stack.pop()
	obj = cls.__new__(cls, *args, **kwargs)
	self.append(obj)

cpython/Modules/_pickle.c

Line 6019 in 4c15505

if (!PyTuple_Check(args)) {

Therefore, if args is an iterable that's not a tuple (like a list), C _pickle will throw an error, but Python pickle will deserialize just fine.

Here are payloads for both opcodes that demonstrate this:

payload:      b'c__main__\nX\n]\x81.'

pickle:       <__main__.X object at 0x7f7411fbd6a0>
_pickle.c:    FAILURE NEWOBJ args argument must be a tuple, not list
pickletools:  
    0: c    GLOBAL     '__main__ X'
    12: ]    EMPTY_LIST
    13: \x81 NEWOBJ
    14: .    STOP
highest protocol among opcodes = 2

payload:      b'c__main__\nX\n]}\x92.'

pickle:       <__main__.X object at 0x7f0a83fd19d0>
_pickle.c:    FAILURE NEWOBJ_EX args argument must be a tuple, not list
pickletools:  
    0: c    GLOBAL     '__main__ X'
    12: ]    EMPTY_LIST
    13: }    EMPTY_DICT
    14: \x92 NEWOBJ_EX
    15: .    STOP
highest protocol among opcodes = 4

I think the easiest solution would be to just add an if not isinstance(args, tuple) check to both Python opcode load functions.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

[serhiy-storchaka]

The isinstance() check would add an overhead.

The code works correctly for valid pickle. It does not detect some errors in invalid pickle, but this is not a bug, it is just a lack of feature. I am not sure that the benefit of the detection is worth the cost.