Check for excess arguments in PyArg_ParseTuple

Author: msimacek

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_method.py

@@ -43,6 +43,15 @@
 __dir__ = __file__.rpartition("/")[0]
 
 
+def assert_raises(err, fn, *args, **kwargs):
+    raised = False
+    try:
+        fn(*args, **kwargs)
+    except err:
+        raised = True
+    assert raised
+
+
 def _reference_classmethod(args):
     if isinstance(args[0], type(list.append)):
         return classmethod(args[0])()
@@ -103,6 +112,16 @@ def test_methods(self):
         assert obj.meth_static_varargs(1, 2, 3) == (None, (1, 2, 3))
         assert obj.meth_static_varargs_keywords(1, 2, 3, a=1, b=2) == (None, (1, 2, 3), {'a': 1, 'b': 2})
 
+        assert_raises(TypeError, obj.meth_noargs, 1)
+        assert_raises(TypeError, obj.meth_o)
+        assert_raises(TypeError, obj.meth_o, 1, 2)
+        assert_raises(TypeError, obj.meth_class_noargs, 1)
+        assert_raises(TypeError, obj.meth_class_o)
+        assert_raises(TypeError, obj.meth_class_o, 1, 2)
+        assert_raises(TypeError, obj.meth_static_noargs, 1)
+        assert_raises(TypeError, obj.meth_static_o)
+        assert_raises(TypeError, obj.meth_static_o, 1, 2)
+
 
 class TestPyMethod(CPyExtTestCase):
     def compile_module(self, name):

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_modsupport.py

@@ -63,7 +63,7 @@ def _reference_typecheck(args, expected_type):
 def _reference_parse_O(args):
     if not isinstance(args[0], tuple) or not isinstance(args[1], dict):
         raise SystemError
-    if args[0]:
+    if len(args[0]) == 1:
         return args[0][0]
     elif "arg0" in args[1]:
         return args[1]["arg0"]
@@ -162,6 +162,7 @@ def compile_module(self, name):
             (tuple(), {"arg1": 'helloworld'}),
             (1, dict()),
             (tuple(), 1),
+            (("a", "excess"), dict()),
         ),
         code='''
         static PyObject* wrap_PyArg_ParseTupleAndKeywords(PyObject* argTuple, PyObject* kwargs) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/common/CExtParseArgumentsNode.java

@@ -155,13 +155,14 @@ int doSpecial(String funName, PTuple argv, Object kwds, @SuppressWarnings("unuse
                         @Cached(value = "getChars(format)", allowUncached = true, dimensions = 1) char[] chars,
                         @Cached("createConvertArgNodes(cachedFormat)") ConvertArgNode[] convertArgNodes,
                         @Cached HashingCollectionNodes.LenNode kwdsLenNode,
+                        @Cached SequenceStorageNodes.LenNode argvLenNode,
                         @Cached PRaiseNativeNode raiseNode) {
             try {
                 PDict kwdsDict = null;
                 if (kwds != null && kwdsLenNode.execute((PDict) kwds) != 0) {
                     kwdsDict = (PDict) kwds;
                 }
-                doParsingExploded(funName, argv, kwdsDict, chars, kwdnames, varargs, nativeConext, convertArgNodes, raiseNode);
+                doParsingExploded(funName, argv, kwdsDict, chars, kwdnames, varargs, nativeConext, convertArgNodes, argvLenNode, raiseNode);
                 return 1;
             } catch (InteropException | ParseArgumentsException e) {
                 return 0;
@@ -173,6 +174,7 @@ int doSpecial(String funName, PTuple argv, Object kwds, @SuppressWarnings("unuse
         int doGeneric(String funName, PTuple argv, Object kwds, String format, Object kwdnames, Object varargs, CExtContext nativeContext,
                         @Cached ConvertArgNode convertArgNode,
                         @Cached HashingCollectionNodes.LenNode kwdsLenNode,
+                        @Cached SequenceStorageNodes.LenNode argvLenNode,
                         @Cached PRaiseNativeNode raiseNode) {
             try {
                 char[] chars = getChars(format);
@@ -184,12 +186,21 @@ int doGeneric(String funName, PTuple argv, Object kwds, String format, Object kw
                 for (int i = 0; i < format.length(); i++) {
                     state = convertArg(state, kwdsDict, chars, i, kwdnames, varargs, convertArgNode, raiseNode);
                 }
+                checkExcessArgs(argv, argvLenNode, state, raiseNode);
                 return 1;
             } catch (InteropException | ParseArgumentsException e) {
                 return 0;
             }
         }
 
+        private static void checkExcessArgs(PTuple argv, SequenceStorageNodes.LenNode argvLenNode, ParserState state, PRaiseNativeNode raiseNode) {
+            int argvLen = argvLenNode.execute(argv.getSequenceStorage());
+            if (argvLen > state.v.argnum) {
+                raiseNode.raiseIntWithoutFrame(0, TypeError, ErrorMessages.EXPECTED_AT_MOST_D_ARGS_GOT_D, state.v.argnum, argvLen);
+                throw ParseArgumentsException.raise();
+            }
+        }
+
         @Fallback
         @SuppressWarnings("unused")
         int error(String funName, Object argv, Object kwds, Object format, Object kwdnames, Object varargs, CExtContext nativeContext,
@@ -199,13 +210,14 @@ int error(String funName, Object argv, Object kwds, Object format, Object kwdnam
 
         @ExplodeLoop(kind = LoopExplosionKind.FULL_UNROLL_UNTIL_RETURN)
         private static void doParsingExploded(String funName, PTuple argv, Object kwds, char[] chars, Object kwdnames, Object varargs, CExtContext nativeContext,
-                        ConvertArgNode[] convertArgNodes, PRaiseNativeNode raiseNode)
+                        ConvertArgNode[] convertArgNodes, SequenceStorageNodes.LenNode argvLenNode, PRaiseNativeNode raiseNode)
                         throws InteropException, ParseArgumentsException {
             CompilerAsserts.partialEvaluationConstant(chars.length);
             ParserState state = new ParserState(funName, new PositionalArgStack(argv, null), nativeContext);
             for (int i = 0; i < chars.length; i++) {
                 state = convertArg(state, kwds, chars, i, kwdnames, varargs, convertArgNodes[i], raiseNode);
             }
+            checkExcessArgs(argv, argvLenNode, state, raiseNode);
         }
 
         private static ParserState convertArg(ParserState state, Object kwds, char[] format, int format_idx, Object kwdnames, Object varargs, ConvertArgNode convertArgNode,