* [PATCH v2 1/2] BaseTools: Fix wrong type of arguments to formatting functions
2022-11-09 0:21 [PATCH v2 0/2] Enable CodeQL Failures and Add New Queries Michael Kubacki
@ 2022-11-09 0:21 ` Michael Kubacki
2022-11-09 0:21 ` [PATCH v2 2/2] edk2.qls: Allow error severity results and add new queries Michael Kubacki
2022-11-09 0:55 ` [PATCH v2 0/2] Enable CodeQL Failures and Add New Queries Michael D Kinney
2 siblings, 0 replies; 4+ messages in thread
From: Michael Kubacki @ 2022-11-09 0:21 UTC (permalink / raw)
To: devel; +Cc: Bob Feng, Liming Gao, Yuwei Chen, Sean Brogan, Michael D Kinney
From: Michael Kubacki <michael.kubacki@microsoft.com>
Fixes issues found with the cpp/wrong-type-format-argument CodeQL
rule in BaseTools.
Reference:
https://cwe.mitre.org/data/definitions/686.html
The following CodeQL errors are resolved:
1. Check failure on line 1115 in
BaseTools/Source/C/EfiRom/EfiRom.c
- This argument should be of type 'int' but is of type 'char *'.
- This argument should be of type 'int' but is of type 'signed
char *'.
2. Check failure on line 359 in
BaseTools/Source/C/GenFw/Elf32Convert.c
- This argument should be of type 'CHAR8 *' but is of type
'unsigned int'.
3. Check failure on line 1841 in
BaseTools/Source/C/GenFw/Elf64Convert.c
- This argument should be of type 'unsigned int' but is of type
'unsigned long long'.
4. Check failure on line 1871 in
BaseTools/Source/C/GenFw/Elf64Convert.c
- This argument should be of type 'unsigned int' but is of type
'unsigned long long'.
5. Check failure on line 2400 in
BaseTools/Source/C/GenFv/GenFvInternalLib.c
- This argument should be of type 'unsigned long long' but is of
type 'unsigned int'.
6. Check failure on line 1099 in
BaseTools/Source/C/GenFw/Elf64Convert.c
- This argument should be of type 'CHAR8 *' but is of type
'unsigned int'.
7. Check failure on line 1098 in
BaseTools/Source/C/GenSec/GenSec.c
- This argument should be of type 'CHAR8 *' but is of type
'char **'.
8. Check failure on line 911 in
BaseTools/Source/C/GenSec/GenSec.c
- This argument should be of type 'CHAR8 *' but is of type
'char **'.
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Sean Brogan <sean.brogan@microsoft.com>
---
BaseTools/Source/C/EfiRom/EfiRom.c | 2 +-
BaseTools/Source/C/GenFv/GenFvInternalLib.c | 2 +-
BaseTools/Source/C/GenFw/Elf32Convert.c | 2 +-
BaseTools/Source/C/GenFw/Elf64Convert.c | 6 +++---
BaseTools/Source/C/GenSec/GenSec.c | 4 ++--
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/BaseTools/Source/C/EfiRom/EfiRom.c b/BaseTools/Source/C/EfiRom/EfiRom.c
index 2506f559d574..fa7bf0e62e6d 100644
--- a/BaseTools/Source/C/EfiRom/EfiRom.c
+++ b/BaseTools/Source/C/EfiRom/EfiRom.c
@@ -1112,7 +1112,7 @@ Routine Description:
goto Done;
}
if (DebugLevel > 9) {
- Error (NULL, 0, 2000, "Invalid option value", "Debug Level range is 0-9, current input level is %d", Argv[1]);
+ Error (NULL, 0, 2000, "Invalid option value", "Debug Level range is 0-9, current input level is %llu", DebugLevel);
ReturnStatus = 1;
goto Done;
}
diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index b5b942500334..6bd59515b1aa 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -2397,7 +2397,7 @@ Routine Description:
VerboseMsg("SecCore entry point Address = 0x%llX", (unsigned long long) SecCoreEntryAddress);
VerboseMsg("BaseAddress = 0x%llX", (unsigned long long) FvInfo->BaseAddress);
bSecCore = (UINT32)(SecCoreEntryAddress - FvInfo->BaseAddress);
- VerboseMsg("offset = 0x%llX", bSecCore);
+ VerboseMsg("offset = 0x%X", bSecCore);
if(bSecCore > 0x0fffff) {
Error(NULL, 0, 3000, "Invalid", "SEC Entry point must be within 1MB of start of the FV");
diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c b/BaseTools/Source/C/GenFw/Elf32Convert.c
index d917a444c82d..87d7f133f132 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -356,7 +356,7 @@ ScanSections32 (
mCoffOffset += sizeof (EFI_IMAGE_NT_HEADERS32);
break;
default:
- VerboseMsg ("%s unknown e_machine type. Assume IA-32", (UINTN)mEhdr->e_machine);
+ VerboseMsg ("%u unknown e_machine type. Assume IA-32", (UINTN)mEhdr->e_machine);
mCoffOffset += sizeof (EFI_IMAGE_NT_HEADERS32);
break;
}
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index c6092269e2d1..8b50774beb1e 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -1096,7 +1096,7 @@ ScanSections64 (
break;
default:
- VerboseMsg ("%s unknown e_machine type. Assume X64", (UINTN)mEhdr->e_machine);
+ VerboseMsg ("%u unknown e_machine type. Assume X64", (UINTN)mEhdr->e_machine);
NtHdr->Pe32Plus.FileHeader.Machine = EFI_IMAGE_MACHINE_X64;
NtHdr->Pe32Plus.OptionalHeader.Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
}
@@ -1837,7 +1837,7 @@ WriteRelocations64 (
case R_X86_64_REX_GOTPCRELX:
break;
case R_X86_64_64:
- VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X",
+ VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08llX",
mCoffSectionsOffset[RelShdr->sh_info] + (Rel->r_offset - SecShdr->sh_addr));
CoffAddFixup(
(UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
@@ -1867,7 +1867,7 @@ WriteRelocations64 (
//
// case R_X86_64_32S:
case R_X86_64_32:
- VerboseMsg ("EFI_IMAGE_REL_BASED_HIGHLOW Offset: 0x%08X",
+ VerboseMsg ("EFI_IMAGE_REL_BASED_HIGHLOW Offset: 0x%08llX",
mCoffSectionsOffset[RelShdr->sh_info] + (Rel->r_offset - SecShdr->sh_addr));
CoffAddFixup(
(UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
diff --git a/BaseTools/Source/C/GenSec/GenSec.c b/BaseTools/Source/C/GenSec/GenSec.c
index a4c2d19aa6f4..cf24d821aa96 100644
--- a/BaseTools/Source/C/GenSec/GenSec.c
+++ b/BaseTools/Source/C/GenSec/GenSec.c
@@ -908,7 +908,7 @@ Routine Description:
if (FileBuffer != NULL) {
free (FileBuffer);
}
- Error (NULL, 0, 2000, "Invalid parameter", "the size of input file %s can't be zero", InputFileName);
+ Error (NULL, 0, 2000, "Invalid parameter", "the size of input file %s can't be zero", *InputFileName);
return EFI_NOT_FOUND;
}
@@ -1095,7 +1095,7 @@ Routine Description:
if (FileBuffer != NULL) {
free (FileBuffer);
}
- Error (NULL, 0, 2000, "Invalid parameter", "the size of input file %s can't be zero", InputFileName);
+ Error (NULL, 0, 2000, "Invalid parameter", "the size of input file %s can't be zero", *InputFileName);
return EFI_NOT_FOUND;
}
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2 0/2] Enable CodeQL Failures and Add New Queries
2022-11-09 0:21 [PATCH v2 0/2] Enable CodeQL Failures and Add New Queries Michael Kubacki
2022-11-09 0:21 ` [PATCH v2 1/2] BaseTools: Fix wrong type of arguments to formatting functions Michael Kubacki
2022-11-09 0:21 ` [PATCH v2 2/2] edk2.qls: Allow error severity results and add new queries Michael Kubacki
@ 2022-11-09 0:55 ` Michael D Kinney
2 siblings, 0 replies; 4+ messages in thread
From: Michael D Kinney @ 2022-11-09 0:55 UTC (permalink / raw)
To: mikuback@linux.microsoft.com, devel@edk2.groups.io,
Kinney, Michael D
Cc: Feng, Bob C, Gao, Liming, Chen, Christine, Sean Brogan
Series Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Tuesday, November 8, 2022 4:22 PM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>; Sean
> Brogan <sean.brogan@microsoft.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH v2 0/2] Enable CodeQL Failures and Add New Queries
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> When CodeQL was enabled, the goal was to enable the flow and not
> impact build results. cpp/conditionallyuninitializedvariable was
> the first and only query enabled with all CodeQL results filtered
> out from affecting CI results.
>
> This achieved the goal to enable CodeQL for future changes to build
> upon but always get CodeQL successful runs in the meantime.
>
> This patch series:
> 1. Swaps out that initial "placeholder" query with two queries that
> can be enabled with no code changes.
> 2. Enables "error" level CodeQL alerts.
> 3. Makes fixes made for a default query
> cpp/wrong-type-format-argument in BaseTools.
>
> The results for (3) can be seen in the following Code Scanning
> results that show the PR with these changes fixed the alerts raised
> by CodeQL.
>
> PR: https://github.com/tianocore/edk2/pull/3617
>
> Code Scanning results (access may be required):
> https://github.com/tianocore/edk2/security/code-scanning?query=pr%3A3617+tool%3ACodeQL+is%3Aclosed
>
> V2 Changes:
>
> 1. Use *InputFileName instead of InputFileName[0] in GenSec.c
> 2. Add R-b tags from v1 series
>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>
> Michael Kubacki (2):
> BaseTools: Fix wrong type of arguments to formatting functions
> edk2.qls: Allow error severity results and add new queries
>
> BaseTools/Source/C/EfiRom/EfiRom.c | 2 +-
> BaseTools/Source/C/GenFv/GenFvInternalLib.c | 2 +-
> BaseTools/Source/C/GenFw/Elf32Convert.c | 2 +-
> BaseTools/Source/C/GenFw/Elf64Convert.c | 6 +++---
> BaseTools/Source/C/GenSec/GenSec.c | 4 ++--
> .github/codeql/codeql-config.yml | 1 -
> .github/codeql/edk2.qls | 4 +++-
> 7 files changed, 11 insertions(+), 10 deletions(-)
>
> --
> 2.28.0.windows.1
^ permalink raw reply [flat|nested] 4+ messages in thread