Memory leak fixes. #66
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixed segfault and fetch problems when checking CRLs
- project is maintained - as wiki is empty, we use its main description (converted to MD) here
... we have a COPYING file anyway fixes some formatting
- Fixed some security issues (thx @frankmorgner): (https://www.x41-dsec.de/lab/advisories/x41-2018-003-pam_pkcs11/) -- fixed buffer overflow with long home directory; -- fixed wiping secrets (now using OpenSSL_cleanse()); -- verify using a nonce from the system, not the card.
this small tweak makes our life easier and should not harm other pam_pkcs11 users. Our build process generates makefiles and objects in dedicated build directory. without this patch it fails with error as follows: xsltproc \ --stringparam section.autolabel 1 \ --stringparam section.label.includes.component.label 1 \ -o pam_pkcs11.html pam_pkcs11.xsl /scratch/sashan/userland/components/pam_pkcs11/pam_pkcs11-0.6.10/doc/pam_pkcs11.xml warning: failed to load external entity "pam_pkcs11.xsl" cannot parse pam_pkcs11.xsl make[3]: *** [Makefile:644: pam_pkcs11.html] Error 4 make[3]: Leaving directory '/scratch/sashan/userland/components/pam_pkcs11/build/i86/doc' make[2]: *** [Makefile:464: all-recursive] Error 1 make[2]: Leaving directory '/scratch/sashan/userland/components/pam_pkcs11/build/i86' make[1]: *** [Makefile:396: all] Error 2 change in this pull request makes my build happy.
Blue text on black background is unreadable. This is the case for login on the console. Green text is readable on a black background and also a white background.
- Solaris runs build process outside of srcdir
Support openssl 1.1.0
card_eventmgr.c: In function ‘main’:
card_eventmgr.c:336:8: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘pid_t’ {aka ‘int’} [-Wformat=]
DBG1("Killing process: %ld", pid);
^~~~~~~~~~~~~~~~~~~~~~ ~~~
pam_pkcs11.c:753:34: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
"PKCS11_LOGIN_TOKEN_NAME=%.*s",
^
pam_pkcs11.c:752:3: note: ‘snprintf’ output between 25 and 256 bytes into a destination of size 255
snprintf(env_temp, sizeof(env_temp) - 1,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"PKCS11_LOGIN_TOKEN_NAME=%.*s",
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
(int)((sizeof(env_temp) - 1) - strlen("PKCS11_LOGIN_TOKEN_NAME=")),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
get_slot_tokenlabel(ph));
~~~~~~~~~~~~~~~~~~~~~~~~
pam_pkcs11.c:770:35: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
"PKCS11_LOGIN_CERT_ISSUER=%.*s",
^
pam_pkcs11.c:769:5: note: ‘snprintf’ output between 26 and 256 bytes into a destination of size 255
snprintf(env_temp, sizeof(env_temp) - 1,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"PKCS11_LOGIN_CERT_ISSUER=%.*s",
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
(int)((sizeof(env_temp) - 1) - strlen("PKCS11_LOGIN_CERT_ISSUER=")),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
issuer[0]);
~~~~~~~~~~
pam_pkcs11.c:792:35: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
"PKCS11_LOGIN_CERT_SERIAL=%.*s",
^
pam_pkcs11.c:791:5: note: ‘snprintf’ output between 26 and 256 bytes into a destination of size 255
snprintf(env_temp, sizeof(env_temp) - 1,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"PKCS11_LOGIN_CERT_SERIAL=%.*s",
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
(int)((sizeof(env_temp) - 1) - strlen("PKCS11_LOGIN_CERT_SERIAL=")),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
serial[0]);
~~~~~~~~~~
The code was incorrect since the addition of OpenSSL 1.1 support in release 0.6.10.
mail_mapper.c: In function ‘compare_email’:
mail_mapper.c:100:27: warning: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
return (at - c_email) == strlen(c_user) && !strncmp(c_email, c_user, strlen(c_user));
^~
ldap_mapper.c:844:33: warning: pointer targets in passing argument 1 of ‘ldap_encode_escapes’ differ in signedness [-Wpointer-sign]
escaped = ldap_encode_escapes(login, strlen(login));
^~~~~
ldap_mapper.c:622:1: note: expected ‘const unsigned char *’ but argument is of type ‘const char *’
ldap_encode_escapes(const unsigned char *binary, size_t length)
^~~~~~~~~~~~~~~~~~~
ldap_mapper.c: In function ‘ldap_build_filter’:
ldap_mapper.c:836:31: warning: declaration of ‘filter’ shadows a global declaration [-Wshadow]
ldap_build_filter(const char *filter, const char *login, const char *map,
~~~~~~~~~~~~^~~~~~
ldap_mapper.c:107:20: note: shadowed declaration is here
static const char *filter="(&(objectClass=posixAccount)(uid=%s)";
^~~~~~
parse.c: In function ‘scconf_parse’: parse.c:389:3: warning: ‘strncpy’ output may be truncated copying 255 bytes from a string of length 255 [-Wstringop-truncation] strncpy(buffer, p.emesg, sizeof(buffer)-1); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ parse.c: In function ‘scconf_parse_string’: parse.c:416:3: warning: ‘strncpy’ output may be truncated copying 255 bytes from a string of length 255 [-Wstringop-truncation] strncpy(buffer, p.emesg, sizeof(buffer)-1); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
memcpy() should be faster than strncpy().
./strings.h: In function ‘clone_str’:
strings.c:53:2: warning: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
strncpy(dst,str,len);
^~~~~~~~~~~~~~~~~~~~
strings.c:50:14: note: length computed here
size_t len= strlen(str);
^~~~~~~~~~~
The function split_static() is not used anywhere in the code.
I could have removed the code but maybe some external mapper are using it?
strings.c:158:9: warning: ‘strncpy’ specified bound depends on the length of the source argument [-Wstringop-overflow=]
strncpy(dst,str,1+strlen(str));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
strings.c:158:27: note: length computed here
strncpy(dst,str,1+strlen(str));
^~~~~~~~~~~
pkcs11_lib.c: In function ‘refresh_slots’:
pkcs11_lib.c:1111:11: warning: declaration of ‘rv’ shadows a previous local [-Wshadow]
CK_RV rv;
^~
pkcs11_lib.c:1056:9: note: shadowed declaration is here
CK_RV rv;
^~
pkcs11_lib.c: In function ‘init_pkcs11_module’:
pkcs11_lib.c:1148:12: warning: unused variable ‘i’ [-Wunused-variable]
CK_ULONG i;
^
null_mapper.c: In function ‘mapper_match_user’:
mapper.h:206:6: warning: declaration of ‘match’ shadows a global declaration [-Wshadow]
int match = 0; \
^~~~~
null_mapper.c:60:1: note: in expansion of macro ‘_DEFAULT_MAPPER_MATCH_USER’
_DEFAULT_MAPPER_MATCH_USER
^~~~~~~~~~~~~~~~~~~~~~~~~~
null_mapper.c:43:12: note: shadowed declaration is here
static int match=0;
^~~~~
openssh_mapper.c: In function ‘openssh_mapper_match_keys’:
openssh_mapper.c:297:27: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
authrsa_e = RSA_get0_e(authrsa);
^
openssh_mapper.c:298:23: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
rsa_e = RSA_get0_e(rsa);
^
openssh_mapper.c:301:27: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
authrsa_n = RSA_get0_n(authrsa);
^
openssh_mapper.c:302:23: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
rsa_n = RSA_get0_n(rsa);
^
This will allow to add support of non-RSA keys. Thanks to sovaeta for the patch OpenSC#23
Thanks to sovaeta got the patch OpenSC#23
The correct length to use is returned by C_Sign() so no need to manually increase the signature length. Thanks to sovaeta for the patch OpenSC#23
Fix compiler error:
cert_vfy.c: At top level:
cert_vfy.c:44:5: error: conflicting types for ‘verify_signature’
int verify_signature(X509 * x509, unsigned char *data, int data_length,
^~~~~~~~~~~~~~~~
In file included from cert_vfy.c:18:0:
cert_vfy.h:81:20: note: previous declaration of ‘verify_signature’ was here
CERTVFY_EXTERN int verify_signature(X509 * x509, unsigned char *data, int data_length, unsigned char **signature, int *signature_length);
^~~~~~~~~~~~~~~~
Fix compiler warning:
pam_pkcs11.c:729:62: warning: passing argument 5 of ‘verify_signature’ from incompatible pointer type [-Wincompatible-pointer-types]
random_value, sizeof(random_value), &signature, &signature_length);
^
In file included from pam_pkcs11.c:45:0:
../common/cert_vfy.h:81:20: note: expected ‘int *’ but argument is of type ‘long unsigned int *’
CERTVFY_EXTERN int verify_signature(X509 * x509, unsigned char *data, int data_length, unsigned char **signature, int *signature_length);
^~~~~~~~~~~~~~~~
|
Hi! Please, look at bowb#1 . |
Updated patches/memleak fixes
|
This pull request introduces 1 alert when merging a5f8e21 into 4efa29f - view on LGTM.com new alerts:
|
Some messages are capitalized but some are not, this commit unifies them by capitalizing all messages, and also appends punctuation.
Just uses `make -C po update-po`, translation should be updated by translators.
Because it happens to be my first language.
Capitalize message
Update specfile to allow build in version 0.6.12
Fix "make distcheck" error: make[3]: *** No rule to make target 'pam.d_ignore_no_card.example', needed by 'all-am'. Stop.
$ doxygen -u doxygen.conf.in
warning: Tag 'TCL_SUBST' at line 237 of file 'doxygen.conf.in' has become obsolete.
This tag has been removed.
warning: Tag 'COLS_IN_ALPHA_INDEX' at line 1021 of file 'doxygen.conf.in' has become obsolete.
This tag has been removed.
warning: Tag 'LATEX_SOURCE_CODE' at line 1723 of file 'doxygen.conf.in' has become obsolete.
This tag has been removed.
warning: Tag 'RTF_SOURCE_CODE' at line 1797 of file 'doxygen.conf.in' has become obsolete.
This tag has been removed.
warning: Tag 'DOCBOOK_PROGRAMLISTING' at line 1895 of file 'doxygen.conf.in' has become obsolete.
This tag has been removed.
warning: Tag 'PERL_PATH' at line 2075 of file 'doxygen.conf.in' has become obsolete.
This tag has been removed.
warning: Tag 'CLASS_DIAGRAMS' at line 2088 of file 'doxygen.conf.in' has become obsolete.
This tag has been removed.
warning: Tag 'MSCGEN_PATH' at line 2097 of file 'doxygen.conf.in' has become obsolete.
This tag has been removed.
Configuration file 'doxygen.conf.in' updated.
Check for spelling errors: doc/mappers_api.xml#L862 ocurrs ==> occurs
Check for spelling errors: doc/doxygen.conf.in#L88 numer ==> number
Check for spelling errors: src/tools/pkcs11_eventmgr.c#L637 sesion ==> session
Check for spelling errors: src/common/base64.h#L2 funtions ==> functions Check for spelling errors: src/common/base64.h#L32 lenght ==> length Check for spelling errors: src/common/base64.h#L35 sucess ==> success
Check for spelling errors: src/common/SSLerrs.h#L241 succesfully ==> successfully
Check for spelling errors: src/common/pkcs11_lib.c#L1646 Sesion ==> Session
|
Bump |
wolneykien
approved these changes
Mar 6, 2023
|
Sorry for keeping you waiting... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix invalid read on from.
Fix memory leaks when using subject mapper.
There are other issues that need addressed. There are memory leaks and other invalid code that needs fixed.
Fixes #65