Housekeeping #8
Housekeeping #8
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cpp-linter Review
Only 2 out of 33 clang-format concerns fit within this pull request's diff.
Click here for the full clang-format patch
diff --git a/pam_shield.h b/pam_shield.h
index 28987cc..c14b054 100644
--- a/pam_shield.h
+++ b/pam_shield.h
@@ -2 +2 @@
- pam_shield.h
+ pam_shield.h
@@ -25 +25 @@
-#define PAM_SHIELD 1
+#define PAM_SHIELD 1
@@ -29,2 +29,2 @@
-#define PAM_SHIELD_ADDR_IPV4 0
-#define PAM_SHIELD_ADDR_IPV6 1
+#define PAM_SHIELD_ADDR_IPV4 0
+#define PAM_SHIELD_ADDR_IPV6 1
@@ -33,11 +33,12 @@ typedef struct {
- unsigned char addr_family; /* PAM_SHIELD_ADDR_IPV4|PAM_SHIELD_ADDR_IPV6 */
- union {
- struct in_addr in; /* IPv4 number */
- struct in6_addr in6; /* IPv6 number */
- char any[1]; /* access to any */
- } ip;
-
- unsigned int max_entries; /* number of timestamps */
- unsigned int count; /* number of auth requests done */
- time_t trigger_active; /* time the trigger was triggered (needed for expiration) */
- time_t timestamps[1]; /* sliding window of timestamps */
+ unsigned char addr_family; /* PAM_SHIELD_ADDR_IPV4|PAM_SHIELD_ADDR_IPV6 */
+ union {
+ struct in_addr in; /* IPv4 number */
+ struct in6_addr in6; /* IPv6 number */
+ char any[1]; /* access to any */
+ } ip;
+
+ unsigned int max_entries; /* number of timestamps */
+ unsigned int count; /* number of auth requests done */
+ time_t trigger_active; /* time the trigger was triggered (needed for
+ expiration) */
+ time_t timestamps[1]; /* sliding window of timestamps */
@@ -46 +46,0 @@ typedef struct {
-
@@ -48,2 +48,2 @@ typedef struct {
- the IP list is used for making in-memory whitelists
- (they are not in the database, but in the config file)
+ the IP list is used for making in-memory whitelists
+ (they are not in the database, but in the config file)
@@ -54,13 +54,13 @@ struct ip_list_tag {
- union {
- struct in_addr in;
- struct in6_addr in6;
- unsigned char any[1];
- } ip;
-
- union {
- struct in_addr in;
- struct in6_addr in6;
- unsigned char any[1];
- } mask;
-
- ip_list *prev, *next;
+ union {
+ struct in_addr in;
+ struct in6_addr in6;
+ unsigned char any[1];
+ } ip;
+
+ union {
+ struct in_addr in;
+ struct in6_addr in6;
+ unsigned char any[1];
+ } mask;
+
+ ip_list *prev, *next;
@@ -73 +73 @@ struct name_list_tag {
- name_list *prev, *next;
+ name_list *prev, *next;
@@ -75 +75 @@ struct name_list_tag {
- char name[1];
+ char name[1];
@@ -78 +78 @@ struct name_list_tag {
-#endif /* PAM_SHIELD */
+#endif /* PAM_SHIELD */
diff --git a/pam_shield_lib.h b/pam_shield_lib.h
index 82c51bf..8a32940 100644
--- a/pam_shield_lib.h
+++ b/pam_shield_lib.h
@@ -2 +2 @@
- pam_shield_lib.h
+ pam_shield_lib.h
@@ -23,0 +24,8 @@
+#include <arpa/inet.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <gdbm.h>
+#include <netdb.h>
+#include <netinet/in.h>
+#include <pwd.h>
+#include <stdarg.h>
@@ -26 +33,0 @@
-#include <stdarg.h>
@@ -28,7 +35 @@
-#include <unistd.h>
-#include <syslog.h>
-#include <pwd.h>
-#include <time.h>
-#include <netdb.h>
-#include <fcntl.h>
-#include <sys/types.h>
+#include <sys/socket.h>
@@ -35,0 +37 @@
+#include <sys/types.h>
@@ -37,5 +39,3 @@
-#include <sys/socket.h>
-#include <netinet/in.h>
-#include <arpa/inet.h>
-#include <errno.h>
-#include <gdbm.h>
+#include <syslog.h>
+#include <time.h>
+#include <unistd.h>
@@ -48,3 +48,3 @@
-#define DEFAULT_MAX_CONNS 10
-#define DEFAULT_INTERVAL 60L
-#define DEFAULT_RETENTION (3600L * 24L)
+#define DEFAULT_MAX_CONNS 10
+#define DEFAULT_INTERVAL 60L
+#define DEFAULT_RETENTION (3600L * 24L)
@@ -52 +52 @@
-#define MAX_LINE 1024
+#define MAX_LINE 1024
@@ -54,9 +54,9 @@
-#define OPT_DEBUG 0x001
-#define OPT_BLOCK_ALL 0x002 /* block all, including known users */
-#define OPT_DRYRUN 0x004
-#define OPT_LISTDB 0x008
-#define OPT_MISSING_DNS 0x010 /* allow missing DNS */
-#define OPT_MISSING_REVERSE 0x020 /* allow missing reverse DNS */
-#define OPT_FORCE 0x040 /* purge unexpired entries */
-#define OPT_REMOVEIP 0x080
-#define OPT_SYNC 0x100
+#define OPT_DEBUG 0x001
+#define OPT_BLOCK_ALL 0x002 /* block all, including known users */
+#define OPT_DRYRUN 0x004
+#define OPT_LISTDB 0x008
+#define OPT_MISSING_DNS 0x010 /* allow missing DNS */
+#define OPT_MISSING_REVERSE 0x020 /* allow missing reverse DNS */
+#define OPT_FORCE 0x040 /* purge unexpired entries */
+#define OPT_REMOVEIP 0x080
+#define OPT_SYNC 0x100
@@ -92,2 +92,2 @@ void add_ip_list(ip_list **root, ip_list *ip);
- try to match an IP number against the allow list
- returns 1 if it matches
+ try to match an IP number against the allow list
+ returns 1 if it matches
@@ -100 +100 @@ int match_ipv6_list(unsigned char *saddr);
- name_lists are hostnames and/or network names
+ name_lists are hostnames and/or network names
@@ -109,2 +109,2 @@ void add_name_list(name_list **root, name_list *n);
- see if 'name' matches our whitelist
- return 1 if it does
+ see if 'name' matches our whitelist
+ return 1 if it does
@@ -114 +113,0 @@ int match_name_list(char *name);
-
@@ -116 +115 @@ int match_name_list(char *name);
- initialize variables
+ initialize variables
@@ -123 +122 @@ void deinit_module(void);
- strip leading and trailing whitespace from a string
+ strip leading and trailing whitespace from a string
@@ -128,11 +127,11 @@ void strip(char *str);
- multipliers:
- 1s second
- 1m minute
- 1h hour
- 1d day
- 1w week
- 1M month
- 1y year
-
- default is 1
- returns 0 on error
+ multipliers:
+ 1s second
+ 1m minute
+ 1h hour
+ 1d day
+ 1w week
+ 1M month
+ 1y year
+
+ default is 1
+ returns 0 on error
@@ -143 +142 @@ long get_multiplier(char *str);
- generate bitmask from '/24' notation
+ generate bitmask from '/24' notation
@@ -145,2 +144,2 @@ long get_multiplier(char *str);
- mask is struct in_addr.saddr, size is the size of the array
- (4 for IPv4, 16 for IPv6)
+ mask is struct in_addr.saddr, size is the size of the array
+ (4 for IPv4, 16 for IPv6)
@@ -151,2 +150,2 @@ void ip_bitmask(int bits, unsigned char *mask, int size);
- allow network/netmask, for both IPv4 and IPv6
- netmask can be in canonical or decimal notation
+ allow network/netmask, for both IPv4 and IPv6
+ netmask can be in canonical or decimal notation
@@ -157 +156 @@ int allow_ip(char *ipnum, int line_no);
- read configuration file
+ read configuration file
@@ -162,2 +161,2 @@ int read_config(void);
- print the IP number of a db_record
- return NULL on error, or buf on success
+ print the IP number of a db_record
+ return NULL on error, or buf on success
@@ -168 +167 @@ const char *print_ip(_pam_shield_db_rec_t *record, char *buf, int buflen);
- run external command
+ run external command
@@ -175 +174 @@ int expire_record(_pam_shield_db_rec_t *record);
- gdbm has encountered a fatal error
+ gdbm has encountered a fatal error
Have any feedback or feature suggestions? Share it here.
| @@ -1,5 +1,24 @@ | |||
| /* | |||
| pam_shield.h | |||
| pam_shield.h | |||
There was a problem hiding this comment.
clang-format suggestions
| pam_shield.h | |
| pam_shield.h |
| @@ -1,9 +1,10 @@ | |||
| /* | |||
| pam_shield_lib.h | |||
| pam_shield_lib.h | |||
There was a problem hiding this comment.
clang-format suggestions
| pam_shield_lib.h | |
| pam_shield_lib.h |
| size); | ||
| return NULL; | ||
| } | ||
| memset(record, 0, size); |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield.c:107:3: warning: [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(record, 0, size);
^~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield.c:107:3: note: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(record, 0, size);
^~~~~~| struct addrinfo hints, *res; | ||
| int err; | ||
|
|
||
| memset(&hints, 0, sizeof(struct addrinfo)); |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield.c:126:3: warning: [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(&hints, 0, sizeof(struct addrinfo));
^~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield.c:126:3: note: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(&hints, 0, sizeof(struct addrinfo));
^~~~~~|
|
||
| /* if not blocking all and the user is known, let go */ | ||
| if (!(options & OPT_BLOCK_ALL) && user != NULL && | ||
| (pwd = getpwnam(user)) != NULL) { |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield.c:173:8: warning: [clang-analyzer-deadcode.DeadStores]
Although the value stored to 'pwd' is used in the enclosing expression, the value is never actually read from 'pwd'
(pwd = getpwnam(user)) != NULL) {
^ ~~~~~~~~~~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield.c:173:8: note: Although the value stored to 'pwd' is used in the enclosing expression, the value is never actually read from 'pwd'
(pwd = getpwnam(user)) != NULL) {
^ ~~~~~~~~~~~~~~| /* try network address as IPv6 */ | ||
| if (inet_pton(AF_INET6, ipnum, &ip->ip.in6) > 0) { | ||
| if (netmask == NULL) { /* no netmask given, treat as host */ | ||
| memset(ip->mask.in6.s6_addr, 0xff, sizeof(ip->mask.in6.s6_addr)); |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield_lib.c:448:7: warning: [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(ip->mask.in6.s6_addr, 0xff, sizeof(ip->mask.in6.s6_addr));
^~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield_lib.c:448:7: note: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(ip->mask.in6.s6_addr, 0xff, sizeof(ip->mask.in6.s6_addr));
^~~~~~| return buf; | ||
| } | ||
| if (record == NULL) { | ||
| strncpy(buf, "(null)", buflen); |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield_lib.c:694:5: warning: [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
Call to function 'strncpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'strncpy_s' in case of C11
strncpy(buf, "(null)", buflen);
^~~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield_lib.c:694:5: note: Call to function 'strncpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'strncpy_s' in case of C11
strncpy(buf, "(null)", buflen);
^~~~~~~|
|
||
| while((err = waitpid(pid, &status, 0)) > 0); | ||
| while ((err = waitpid(pid, &status, 0)) > 0) |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield_lib.c:748:13: warning: [clang-analyzer-deadcode.DeadStores]
Although the value stored to 'err' is used in the enclosing expression, the value is never actually read from 'err'
while ((err = waitpid(pid, &status, 0)) > 0)
^ ~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield_lib.c:748:13: note: Although the value stored to 'err' is used in the enclosing expression, the value is never actually read from 'err'
while ((err = waitpid(pid, &status, 0)) > 0)
^ ~~~~~~~~~~~~~~~~~~~~~~~~| */ | ||
| while (record->count > 0 && | ||
| difftime(this_time, record->timestamps[0]) >= (double)interval) { | ||
| memmove(record->timestamps, &record->timestamps[1], |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield_lib.c:770:5: warning: [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
Call to function 'memmove' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memmove_s' in case of C11
memmove(record->timestamps, &record->timestamps[1],
^~~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield_lib.c:770:5: note: Call to function 'memmove' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memmove_s' in case of C11
memmove(record->timestamps, &record->timestamps[1],
^~~~~~~| key = next_key; | ||
| if (deleted && !key.dptr) { | ||
| if (!(options & OPT_DRYRUN)) { | ||
| key = gdbm_firstkey(dbf); |
There was a problem hiding this comment.
clang-tidy diagnostic
shield_purge.c:304:9: warning: [clang-analyzer-deadcode.DeadStores]
Value stored to 'key' is never read
key = gdbm_firstkey(dbf);
^ ~~~~~~~~~~~~~~~~~~
/home/runner/work/pam_shield/pam_shield/shield_purge.c:304:9: note: Value stored to 'key' is never read
key = gdbm_firstkey(dbf);
^ ~~~~~~~~~~~~~~~~~~
There was a problem hiding this comment.
Cpp-linter Review
Click here for the full clang-format patch
diff --git a/pam_shield.h b/pam_shield.h
index ec106dd..ec24733 100644
--- a/pam_shield.h
+++ b/pam_shield.h
@@ -2 +2 @@
- pam_shield.h
+ pam_shield.h
diff --git a/pam_shield_lib.h b/pam_shield_lib.h
index 5189b09..35a82ff 100644
--- a/pam_shield_lib.h
+++ b/pam_shield_lib.h
@@ -2 +2 @@
- pam_shield_lib.h
+ pam_shield_lib.h
Have any feedback or feature suggestions? Share it here.
| @@ -1,61 +1,78 @@ | |||
| /* | |||
| pam_shield.h | |||
| pam_shield.h | |||
There was a problem hiding this comment.
clang-format suggestions
| pam_shield.h | |
| pam_shield.h |
| @@ -1,9 +1,10 @@ | |||
| /* | |||
| pam_shield_lib.h | |||
| pam_shield_lib.h | |||
There was a problem hiding this comment.
clang-format suggestions
| pam_shield_lib.h | |
| pam_shield_lib.h |
| size); | ||
| return NULL; | ||
| } | ||
| memset(record, 0, size); |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield.c:102:3: warning: [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(record, 0, size);
^~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield.c:102:3: note: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(record, 0, size);
^~~~~~| struct addrinfo hints, *res; | ||
| int err; | ||
|
|
||
| memset(&hints, 0, sizeof(struct addrinfo)); |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield.c:121:3: warning: [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(&hints, 0, sizeof(struct addrinfo));
^~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield.c:121:3: note: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(&hints, 0, sizeof(struct addrinfo));
^~~~~~|
|
||
| /* if not blocking all and the user is known, let go */ | ||
| if (!(options & OPT_BLOCK_ALL) && user != NULL && | ||
| (pwd = getpwnam(user)) != NULL) { |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield.c:168:8: warning: [clang-analyzer-deadcode.DeadStores]
Although the value stored to 'pwd' is used in the enclosing expression, the value is never actually read from 'pwd'
(pwd = getpwnam(user)) != NULL) {
^ ~~~~~~~~~~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield.c:168:8: note: Although the value stored to 'pwd' is used in the enclosing expression, the value is never actually read from 'pwd'
(pwd = getpwnam(user)) != NULL) {
^ ~~~~~~~~~~~~~~| /* try network address as IPv6 */ | ||
| if (inet_pton(AF_INET6, ipnum, &ip->ip.in6) > 0) { | ||
| if (netmask == NULL) { /* no netmask given, treat as host */ | ||
| memset(ip->mask.in6.s6_addr, 0xff, sizeof(ip->mask.in6.s6_addr)); |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield_lib.c:442:7: warning: [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(ip->mask.in6.s6_addr, 0xff, sizeof(ip->mask.in6.s6_addr));
^~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield_lib.c:442:7: note: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(ip->mask.in6.s6_addr, 0xff, sizeof(ip->mask.in6.s6_addr));
^~~~~~| return buf; | ||
| } | ||
| if (record == NULL) { | ||
| strncpy(buf, "(null)", buflen); |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield_lib.c:686:5: warning: [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
Call to function 'strncpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'strncpy_s' in case of C11
strncpy(buf, "(null)", buflen);
^~~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield_lib.c:686:5: note: Call to function 'strncpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'strncpy_s' in case of C11
strncpy(buf, "(null)", buflen);
^~~~~~~|
|
||
| while((err = waitpid(pid, &status, 0)) > 0); | ||
| while ((err = waitpid(pid, &status, 0)) > 0) |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield_lib.c:738:13: warning: [clang-analyzer-deadcode.DeadStores]
Although the value stored to 'err' is used in the enclosing expression, the value is never actually read from 'err'
while ((err = waitpid(pid, &status, 0)) > 0)
^ ~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield_lib.c:738:13: note: Although the value stored to 'err' is used in the enclosing expression, the value is never actually read from 'err'
while ((err = waitpid(pid, &status, 0)) > 0)
^ ~~~~~~~~~~~~~~~~~~~~~~~~| /* expire entries that are no longer in this interval (sliding window) */ | ||
| while (record->count > 0 && | ||
| difftime(this_time, record->timestamps[0]) >= (double)interval) { | ||
| memmove(record->timestamps, &record->timestamps[1], |
There was a problem hiding this comment.
clang-tidy diagnostic
pam_shield_lib.c:758:5: warning: [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
Call to function 'memmove' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memmove_s' in case of C11
memmove(record->timestamps, &record->timestamps[1],
^~~~~~~
/home/runner/work/pam_shield/pam_shield/pam_shield_lib.c:758:5: note: Call to function 'memmove' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memmove_s' in case of C11
memmove(record->timestamps, &record->timestamps[1],
^~~~~~~| key = next_key; | ||
| if (deleted && !key.dptr) { | ||
| if (!(options & OPT_DRYRUN)) { | ||
| key = gdbm_firstkey(dbf); |
There was a problem hiding this comment.
clang-tidy diagnostic
shield_purge.c:298:9: warning: [clang-analyzer-deadcode.DeadStores]
Value stored to 'key' is never read
key = gdbm_firstkey(dbf);
^ ~~~~~~~~~~~~~~~~~~
/home/runner/work/pam_shield/pam_shield/shield_purge.c:298:9: note: Value stored to 'key' is never read
key = gdbm_firstkey(dbf);
^ ~~~~~~~~~~~~~~~~~~
No description provided.