Skip to content

[DRAFT] Add EVP_read_pw_string #2402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 19 commits into from
Closed

Conversation

smittals2
Copy link
Contributor

Issues:

Resolves #ISSUE-NUMBER1
Addresses #ISSUE-NUMBER2

Description of changes:

Describe AWS-LC’s current behavior and how your code changes that behavior. If there are no issues this pr is resolving, explain why this change is necessary.

Call-outs:

Point out areas that need special attention or support during the review process. Discuss architecture or design changes.

Testing:

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@smittals2 smittals2 changed the title Add EVP_read_pw_string [DRAFT] Add EVP_read_pw_string May 8, 2025
@codecov-commenter
Copy link

codecov-commenter commented May 8, 2025

Codecov Report

Attention: Patch coverage is 68.15920% with 64 lines in your changes missing coverage. Please review.

Project coverage is 78.81%. Comparing base (7d9cbf3) to head (5ce4f87).
Report is 27 commits behind head on main.

Files with missing lines Patch % Lines
crypto/fipsmodule/evp/evp.c 0.00% 29 Missing ⚠️
crypto/pem/pem_passwd.c 81.18% 19 Missing ⚠️
crypto/pem/pem_lib.c 0.00% 14 Missing ⚠️
crypto/pem/pem_test.cc 96.49% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2402      +/-   ##
==========================================
+ Coverage   78.76%   78.81%   +0.05%     
==========================================
  Files         620      622       +2     
  Lines      107961   108599     +638     
  Branches    15330    15416      +86     
==========================================
+ Hits        85032    85593     +561     
- Misses      22274    22333      +59     
- Partials      655      673      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 125. Check the log or trigger a new build to see more.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 114. Check the log or trigger a new build to see more.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 108. Check the log or trigger a new build to see more.

#define NUM_SIG 32

DEFINE_BSS_GET(sig_atomic_t, intr_signal);
DEFINE_STATIC_MUTEX(console_global_mutex);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'console_global_mutex' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_STATIC_MUTEX(console_global_mutex);
                    ^

DEFINE_STATIC_MUTEX(console_global_mutex);

# ifdef SIGACTION
static struct sigaction savsig[NUM_SIG];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'savsig' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

    static struct sigaction savsig[NUM_SIG];
                            ^

static void (*savsig[NUM_SIG]) (int);
# endif

static int read_till_nl(FILE *);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
static int read_till_nl(FILE *);
static int read_till_nl(FILE * /*in*/);

# endif

static int read_till_nl(FILE *);
static void recsig(int);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
static void recsig(int);
static void recsig(int /*signal*/);

DEFINE_BSS_GET(DWORD, tty_orig);
DEFINE_BSS_GET(DWORD, tty_new);
#else
DEFINE_BSS_GET(TTY_STRUCT, tty_orig);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_orig' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

    DEFINE_BSS_GET(TTY_STRUCT, tty_orig);
                               ^

DEFINE_BSS_GET(DWORD, tty_new);
#else
DEFINE_BSS_GET(TTY_STRUCT, tty_orig);
DEFINE_BSS_GET(TTY_STRUCT, tty_new);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_new' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

    DEFINE_BSS_GET(TTY_STRUCT, tty_new);
                               ^

DEFINE_BSS_GET(TTY_STRUCT, tty_new);
#endif

DEFINE_BSS_GET(FILE*, tty_in);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_in' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_BSS_GET(FILE*, tty_in);
                      ^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 98. Check the log or trigger a new build to see more.

DEFINE_BSS_GET(TTY_STRUCT, tty_new);
#endif

DEFINE_BSS_GET(FILE*, tty_in);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_in' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_BSS_GET(FILE*, tty_in);
                      ^

#endif

DEFINE_BSS_GET(FILE*, tty_in);
DEFINE_BSS_GET(FILE*, tty_out);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_out' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_BSS_GET(FILE*, tty_out);
                      ^

#endif

DEFINE_BSS_GET(FILE*, tty_in);
DEFINE_BSS_GET(FILE*, tty_out);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_out' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_BSS_GET(FILE*, tty_out);
                      ^


DEFINE_BSS_GET(FILE*, tty_in);
DEFINE_BSS_GET(FILE*, tty_out);
DEFINE_BSS_GET(int, is_a_tty);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'is_a_tty' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_BSS_GET(int, is_a_tty);
                    ^

DEFINE_BSS_GET(int, is_a_tty);

/* Internal functions to read a string without echoing */
static int read_till_nl(FILE *in) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter name 'in' is too short, expected at least 3 characters [readability-identifier-length]

static int read_till_nl(FILE *in) {
                              ^


do {
if (!fgets(buf, SIZE, in))
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: statement should be inside braces [readability-braces-around-statements]

Suggested change
return 0;
if (!fgets(buf, SIZE, in)) {
return 0;
}

/* Signal handling functions */
static void pushsig(void) {
# if !defined(_WIN32)
int i;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'i' is not initialized [cppcoreguidelines-init-variables]

Suggested change
int i;
int i = 0;

/* Signal handling functions */
static void pushsig(void) {
# if !defined(_WIN32)
int i;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable name 'i' is too short, expected at least 3 characters [readability-identifier-length]

    int i;
        ^

static void pushsig(void) {
# if !defined(_WIN32)
int i;
struct sigaction sa;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable name 'sa' is too short, expected at least 3 characters [readability-identifier-length]

    struct sigaction sa;
                     ^

int i;
struct sigaction sa;

memset(&sa, 0, sizeof(sa));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 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 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]

    memset(&sa, 0, sizeof(sa));
    ^
Additional context

crypto/pem/pem_passwd.c:80: 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(&sa, 0, sizeof(sa));
    ^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 88. Check the log or trigger a new build to see more.


static void popsig(void) {
# if !defined(_WIN32)
int i;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'i' is not initialized [cppcoreguidelines-init-variables]

Suggested change
int i;
int i = 0;


static void popsig(void) {
# if !defined(_WIN32)
int i;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable name 'i' is too short, expected at least 3 characters [readability-identifier-length]

    int i;
        ^

*is_a_tty_bss_get() = 1;

# if !defined(_WIN32)
if ((*tty_in_bss_get() = fopen(DEV_TTY, "r")) == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]

    if ((*tty_in_bss_get() = fopen(DEV_TTY, "r")) == NULL) {
                           ^
Additional context

crypto/pem/pem_passwd.c:134: if it should be an assignment, move it out of the 'if' condition

    if ((*tty_in_bss_get() = fopen(DEV_TTY, "r")) == NULL) {
                           ^

crypto/pem/pem_passwd.c:134: if it is meant to be an equality check, change '=' to '=='

    if ((*tty_in_bss_get() = fopen(DEV_TTY, "r")) == NULL) {
                           ^

if ((*tty_in_bss_get() = fopen(DEV_TTY, "r")) == NULL) {
*tty_in_bss_get() = stdin;
}
if ((*tty_out_bss_get() = fopen(DEV_TTY, "w")) == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]

    if ((*tty_out_bss_get() = fopen(DEV_TTY, "w")) == NULL) {
                            ^
Additional context

crypto/pem/pem_passwd.c:137: if it should be an assignment, move it out of the 'if' condition

    if ((*tty_out_bss_get() = fopen(DEV_TTY, "w")) == NULL) {
                            ^

crypto/pem/pem_passwd.c:137: if it is meant to be an equality check, change '=' to '=='

    if ((*tty_out_bss_get() = fopen(DEV_TTY, "w")) == NULL) {
                            ^


static int openssl_console_echo_disable(void) {
# if !defined(_WIN32)
memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Call to function 'memcpy' 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 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]

    memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
    ^
Additional context

crypto/pem/pem_passwd.c:188: Call to function 'memcpy' 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 'memcpy_s' in case of C11

    memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
    ^


static int openssl_console_echo_enable(void) {
# if !defined(_WIN32)
memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Call to function 'memcpy' 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 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]

    memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
    ^
Additional context

crypto/pem/pem_passwd.c:206: Call to function 'memcpy' 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 'memcpy_s' in case of C11

    memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
    ^

}

// returns 0 on success, -1 on error, -2 if interrupt signal received
int openssl_console_read(char *buf, int minsize, int maxsize, int echo) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 3 adjacent parameters of 'openssl_console_read' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]

int openssl_console_read(char *buf, int minsize, int maxsize, int echo) {
                                    ^
Additional context

crypto/pem/pem_passwd.c:227: the first parameter in the range is 'minsize'

int openssl_console_read(char *buf, int minsize, int maxsize, int echo) {
                                        ^

crypto/pem/pem_passwd.c:227: the last parameter in the range is 'echo'

int openssl_console_read(char *buf, int minsize, int maxsize, int echo) {
                                                                  ^


// returns 0 on success, -1 on error, -2 if interrupt signal received
int openssl_console_read(char *buf, int minsize, int maxsize, int echo) {
int ok = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable name 'ok' is too short, expected at least 3 characters [readability-identifier-length]

    int ok = 0;
        ^

// returns 0 on success, -1 on error, -2 if interrupt signal received
int openssl_console_read(char *buf, int minsize, int maxsize, int echo) {
int ok = 0;
char *p = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable name 'p' is too short, expected at least 3 characters [readability-identifier-length]

    char *p = NULL;
          ^

}

// check if we see a new line, otherwise clear out remaining input buffer
if ((p = strchr(buf, '\n')) != NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]

    if ((p = strchr(buf, '\n')) != NULL) {
           ^
Additional context

crypto/pem/pem_passwd.c:272: if it should be an assignment, move it out of the 'if' condition

    if ((p = strchr(buf, '\n')) != NULL) {
           ^

crypto/pem/pem_passwd.c:272: if it is meant to be an equality check, change '=' to '=='

    if ((p = strchr(buf, '\n')) != NULL) {
           ^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 87. Check the log or trigger a new build to see more.


#define NUM_SIG 32

DEFINE_BSS_GET(sig_atomic_t, intr_signal)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'intr_signal' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_BSS_GET(sig_atomic_t, intr_signal)
                             ^

#define NUM_SIG 32

DEFINE_BSS_GET(sig_atomic_t, intr_signal)
DEFINE_STATIC_MUTEX(console_global_mutex)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'console_global_mutex' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_STATIC_MUTEX(console_global_mutex)
                    ^

DEFINE_BSS_GET(DWORD, tty_orig);
DEFINE_BSS_GET(DWORD, tty_new);
#else
DEFINE_BSS_GET(TTY_STRUCT, tty_orig)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_orig' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

    DEFINE_BSS_GET(TTY_STRUCT, tty_orig)
                               ^

DEFINE_BSS_GET(DWORD, tty_new);
#else
DEFINE_BSS_GET(TTY_STRUCT, tty_orig)
DEFINE_BSS_GET(TTY_STRUCT, tty_new)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_new' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

    DEFINE_BSS_GET(TTY_STRUCT, tty_new)
                               ^

DEFINE_BSS_GET(TTY_STRUCT, tty_new)
#endif

DEFINE_BSS_GET(FILE*, tty_in)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_in' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_BSS_GET(FILE*, tty_in)
                      ^

DEFINE_BSS_GET(TTY_STRUCT, tty_new)
#endif

DEFINE_BSS_GET(FILE*, tty_in)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_in' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_BSS_GET(FILE*, tty_in)
                      ^

#endif

DEFINE_BSS_GET(FILE*, tty_in)
DEFINE_BSS_GET(FILE*, tty_out)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_out' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_BSS_GET(FILE*, tty_out)
                      ^

#endif

DEFINE_BSS_GET(FILE*, tty_in)
DEFINE_BSS_GET(FILE*, tty_out)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tty_out' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_BSS_GET(FILE*, tty_out)
                      ^


DEFINE_BSS_GET(FILE*, tty_in)
DEFINE_BSS_GET(FILE*, tty_out)
DEFINE_BSS_GET(int, is_a_tty)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'is_a_tty' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

DEFINE_BSS_GET(int, is_a_tty)
                    ^

ok = -2; // interrupted
}
if (echo_eol) {
fprintf(*tty_out_bss_get(), "\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Call to function 'fprintf' 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 'fprintf_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]

        fprintf(*tty_out_bss_get(), "\n");
        ^
Additional context

crypto/pem/pem_passwd.c:290: Call to function 'fprintf' 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 'fprintf_s' in case of C11

        fprintf(*tty_out_bss_get(), "\n");
        ^

@smittals2 smittals2 closed this May 13, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 86. Check the log or trigger a new build to see more.

typedef struct {
struct sigaction array[NUM_SIG];
} sigaction_array_t;
DEFINE_BSS_GET(sigaction_array_t, savsig)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'savsig' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

    DEFINE_BSS_GET(sigaction_array_t, savsig)
                                      ^

int i;
struct sigaction sa;

memset(&sa, 0, sizeof(sa));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 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 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]

    memset(&sa, 0, sizeof(sa));
    ^
Additional context

crypto/pem/pem_passwd.c:86: 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(&sa, 0, sizeof(sa));
    ^

*is_a_tty_bss_get() = 1;

# if !defined(_WIN32)
if ((*tty_in_bss_get() = fopen(DEV_TTY, "r")) == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]

    if ((*tty_in_bss_get() = fopen(DEV_TTY, "r")) == NULL) {
                           ^
Additional context

crypto/pem/pem_passwd.c:140: if it should be an assignment, move it out of the 'if' condition

    if ((*tty_in_bss_get() = fopen(DEV_TTY, "r")) == NULL) {
                           ^

crypto/pem/pem_passwd.c:140: if it is meant to be an equality check, change '=' to '=='

    if ((*tty_in_bss_get() = fopen(DEV_TTY, "r")) == NULL) {
                           ^

if ((*tty_in_bss_get() = fopen(DEV_TTY, "r")) == NULL) {
*tty_in_bss_get() = stdin;
}
if ((*tty_out_bss_get() = fopen(DEV_TTY, "w")) == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]

    if ((*tty_out_bss_get() = fopen(DEV_TTY, "w")) == NULL) {
                            ^
Additional context

crypto/pem/pem_passwd.c:143: if it should be an assignment, move it out of the 'if' condition

    if ((*tty_out_bss_get() = fopen(DEV_TTY, "w")) == NULL) {
                            ^

crypto/pem/pem_passwd.c:143: if it is meant to be an equality check, change '=' to '=='

    if ((*tty_out_bss_get() = fopen(DEV_TTY, "w")) == NULL) {
                            ^


static int openssl_console_echo_disable(void) {
# if !defined(_WIN32)
memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Call to function 'memcpy' 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 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]

    memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
    ^
Additional context

crypto/pem/pem_passwd.c:194: Call to function 'memcpy' 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 'memcpy_s' in case of C11

    memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
    ^


static int openssl_console_echo_enable(void) {
# if !defined(_WIN32)
memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Call to function 'memcpy' 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 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]

    memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
    ^
Additional context

crypto/pem/pem_passwd.c:212: Call to function 'memcpy' 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 'memcpy_s' in case of C11

    memcpy(tty_new_bss_get(), tty_orig_bss_get(), sizeof(*tty_orig_bss_get()));
    ^

}

// returns 0 on success, -1 on error, -2 if interrupt signal received
int openssl_console_read(char *buf, int minsize, int maxsize, int echo) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 3 adjacent parameters of 'openssl_console_read' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]

int openssl_console_read(char *buf, int minsize, int maxsize, int echo) {
                                    ^
Additional context

crypto/pem/pem_passwd.c:233: the first parameter in the range is 'minsize'

int openssl_console_read(char *buf, int minsize, int maxsize, int echo) {
                                        ^

crypto/pem/pem_passwd.c:233: the last parameter in the range is 'echo'

int openssl_console_read(char *buf, int minsize, int maxsize, int echo) {
                                                                  ^

}

// check if we see a new line, otherwise clear out remaining input buffer
if ((p = strchr(buf, '\n')) != NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]

    if ((p = strchr(buf, '\n')) != NULL) {
           ^
Additional context

crypto/pem/pem_passwd.c:278: if it should be an assignment, move it out of the 'if' condition

    if ((p = strchr(buf, '\n')) != NULL) {
           ^

crypto/pem/pem_passwd.c:278: if it is meant to be an equality check, change '=' to '=='

    if ((p = strchr(buf, '\n')) != NULL) {
           ^

ok = -2; // interrupted
}
if (echo_eol) {
fprintf(*tty_out_bss_get(), "\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Call to function 'fprintf' 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 'fprintf_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]

        fprintf(*tty_out_bss_get(), "\n");
        ^
Additional context

crypto/pem/pem_passwd.c:296: Call to function 'fprintf' 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 'fprintf_s' in case of C11

        fprintf(*tty_out_bss_get(), "\n");
        ^

@@ -15,6 +15,16 @@
#include <openssl/pem.h>

#include <gtest/gtest.h>
#include <signal.h>
#if defined(_WIN32)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: included header signal.h is not used directly [misc-include-cleaner]

Suggested change
#if defined(_WIN32)
#if defined(_WIN32)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants