Skip to content

Code review pal#194

Merged
JPeterMugaas merged 7 commits intomainfrom
code-review-pal
Mar 3, 2026
Merged

Code review pal#194
JPeterMugaas merged 7 commits intomainfrom
code-review-pal

Conversation

@JPeterMugaas
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Contributor

@tregubovav-dev tregubovav-dev left a comment

Choose a reason for hiding this comment

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

All PAL warning suspended with this PR are false-positive warnings.
However, there are a couple issues not related to PAL needs to be reviewed and resolved.
See comments below.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this function needs additional failure checks:

function LoadCertificate(const AFileName: String): PX509;
var
  LM: TMemoryStream;   //PALOFF - Created and freed objects,  Local identifiers that possibly are set more than once without referencing in-between
  LB: PBIO;
begin
  LB:=nil;
  LM := TMemoryStream.Create;
  try
    LM.LoadFromFile(AFileName);
    LB := BIO_new_mem_buf(LM.Memory^, LM.Size); //BIO_new_mem_buf may return NULL value
    Result := PEM_read_bio_X509(LB, nil, nil, nil); //Is it safe to pass NULL BIO to this routine?
  finally
    BIO_free(LB);
    FreeAndNil(LM);
  end;
end;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For example, the 'TaurusTLS_SSL_CTX_use_PrivateKey_file_PKCS12, TaurusTLS_unicode_X509_load_cert_file`, etc. include additional error control to avoid NULL-referenced BIO.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

method function TaurusTLS_unicode_X509_load_cert_crl_file:
Variable Linf can reach line 596 uninitialized.
It must be null-assigned explicitly in the beginning of method.

@JPeterMugaas JPeterMugaas merged commit f264e1d into main Mar 3, 2026
@JPeterMugaas JPeterMugaas deleted the code-review-pal branch March 3, 2026 10:21
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