Issue 35050: Off-by-one bug in AF_ALG

The error checking code for salg_name and salg_type have an off-by-one bug. It should check that both strings are NUL terminated strings. It's not a security bug, because the Linux kernel ensures that the last byte is a NULL byte.

Christian and me created a bug report at the same time :-) My message:

I found two interesting warnings on socketmodule.c in the Coverity report:

Error: BUFFER_SIZE_WARNING (CWE-120): [#def12]
Python-3.6.5/Modules/socketmodule.c:2069: buffer_size_warning: Calling strncpy with a maximum size argument of 14 bytes on destination array "sa->salg_type" of size 14 bytes might leave the destination string unterminated.
# 2067|               return 0;
# 2068|           }
# 2069|->         strncpy((char *)sa->salg_type, type, sizeof(sa->salg_type));
# 2070|           if (strlen(name) > sizeof(sa->salg_name)) {
# 2071|               PyErr_SetString(PyExc_ValueError, "AF_ALG name too long.");

Error: BUFFER_SIZE_WARNING (CWE-120): [#def13]
Python-3.6.5/Modules/socketmodule.c:2074: buffer_size_warning: Calling strncpy with a maximum size argument of 64 bytes on destination array "sa->salg_name" of size 64 bytes might leave the destination string unterminated.
# 2072|               return 0;
# 2073|           }
# 2074|->         strncpy((char *)sa->salg_name, name, sizeof(sa->salg_name));
# 2075|   
# 2076|           *len_ret = sizeof(*sa);


It seems like the Linux kernel always write a terminating NUL byte for AF_ALG:
https://elixir.bootlin.com/linux/latest/source/crypto/af_alg.c#L171

The Python code does not create buffer overflow, it's just that the Linux kernel will always reject names which are too long. Python should reject them as well.

> The Python code does not create buffer overflow, it's just that the Linux kernel will always reject names which are too long.

The Kernel doesn't have a direct length restriction. It just ensures that type and name are NULL terminated. Other code inside the Kernel rejects unknown type and name values.

The error checking code for salg_name and salg_type have an off-by-one bug. Must check that both strings are NUL terminated strings.

New changeset 2eb6ad8578fa9d764c21a92acd8e054e3202ad19 by Victor Stinner (Christian Heimes) in branch 'master':
bpo-35050: AF_ALG length check off-by-one error (GH-10058)
https://github.com/python/cpython/commit/2eb6ad8578fa9d764c21a92acd8e054e3202ad19

New changeset bad41cefef6625807198a813d9dec2c08d59dc60 by Victor Stinner in branch '3.6':
bpo-35050: AF_ALG length check off-by-one error (GH-10058) (GH-11070)
https://github.com/python/cpython/commit/bad41cefef6625807198a813d9dec2c08d59dc60

New changeset 1a7b62d5571b3742e706d247dfe6509f68f1409d by Victor Stinner in branch '3.7':
bpo-35050: AF_ALG length check off-by-one error (GH-10058) (GH-11069)
https://github.com/python/cpython/commit/1a7b62d5571b3742e706d247dfe6509f68f1409d

Thanks for the fix Christian!

Note: Python 2 is not affected, it doesn't support AF_ALG.

stage: patch review -> resolved