Skip to content
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

Fix memory leaks in PE format #4727

Merged
merged 1 commit into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions librz/bin/format/pe/pe.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ struct PE_(rz_bin_pe_obj_t) {
Sdb *kv;
RzCMS *cms;
RzSpcIndirectDataContent *spcinfo;
bool has_canary;
char *authentihash;
bool is_authhash_valid;
bool is_signed;
Expand Down Expand Up @@ -244,6 +245,7 @@ int PE_(rz_bin_pe_is_stripped_local_syms)(RzBinPEObj *bin);
int PE_(rz_bin_pe_is_stripped_debug)(RzBinPEObj *bin);
int PE_(bin_pe_get_claimed_checksum)(RzBinPEObj *bin);
int PE_(bin_pe_get_actual_checksum)(RzBinPEObj *bin);
bool PE_(rz_bin_pe_has_canary)(const RzBinPEObj *bin);
struct rz_bin_pe_addr_t *PE_(check_unknow)(RzBinPEObj *bin);
struct rz_bin_pe_addr_t *PE_(check_msvcseh)(RzBinPEObj *bin);
struct rz_bin_pe_addr_t *PE_(check_mingw)(RzBinPEObj *bin);
Expand Down
4 changes: 4 additions & 0 deletions librz/bin/format/pe/pe_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ static inline int is_arm(RzBinPEObj *bin) {
return 0;
}

bool PE_(rz_bin_pe_has_canary)(const RzBinPEObj *bin) {
return bin->has_canary;
}

// TODO: make it const! like in elf
char *PE_(rz_bin_pe_get_machine)(RzBinPEObj *bin) {
char *machine = NULL;
Expand Down
2 changes: 1 addition & 1 deletion librz/bin/format/pe/pe_relocs.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ int PE_(bin_pe_init_relocs)(RZ_NONNULL RzBinPEObj *bin) {
}

if (PE_(rz_bin_pe_is_stripped_relocs)(bin) || !get_relocs_from_data_dir(bin, ret)) {
rz_vector_free(bin->relocs);
rz_vector_free(ret);
bin->relocs = NULL;
return false;
}
Expand Down
35 changes: 14 additions & 21 deletions librz/bin/p/bin_pe.inc
Original file line number Diff line number Diff line change
Expand Up @@ -429,14 +429,17 @@ static RzPVector /*<RzBinImport *>*/ *imports(RzBinFile *bf) {
if (!bf || !bf->o || !bf->o->bin_obj) {
return NULL;
}
RzBinPEObj *bin = (RzBinPEObj *)bf->o->bin_obj;
bin->has_canary = false;
RzPVector *ret = rz_pvector_new((RzListFree)rz_bin_import_free);
if (!ret) {
return NULL;
}

if (!(imports = PE_(rz_bin_pe_get_imports)(bf->o->bin_obj))) {
if (!(imports = PE_(rz_bin_pe_get_imports)(bin))) {
return ret;
}
bool has_canary = false;
for (i = 0; !imports[i].last; i++) {
if (!(ptr = RZ_NEW0(RzBinImport))) {
break;
Expand All @@ -451,7 +454,15 @@ static RzPVector /*<RzBinImport *>*/ *imports(RzBinFile *bf) {
// index for the import. There is no point in exposing it.
// ptr->hint = imports[i].hint;
rz_pvector_push(ret, ptr);

// __security_init_cookie is a function imported from msvcrt.dll (libc) that when called
// initiliazes the stack canary. So if the function is imported, we can
// conclude the binary uses stack canary.
if (!has_canary && !strcmp(ptr->name, "__security_init_cookie")) {
has_canary = true;
}
}
bin->has_canary = has_canary;
free(imports);
return ret;
}
Expand Down Expand Up @@ -637,24 +648,6 @@ err:
return NULL;
}

static int has_canary(RzBinFile *bf) {
void **it;
const RzPVector *imports_vec = imports(bf);
RzBinImport *imp;
if (imports_vec) {
rz_pvector_foreach (imports_vec, it) {
imp = *it;
// __security_init_cookie is a function imported from msvcrt.dll (libc) that when called
// initiliazes the stack canary. So if the function is imported, we can
// assume the binary uses stack canary.
if (!strcmp(imp->name, "__security_init_cookie")) {
return true;
}
}
}
return false;
}

static inline bool haschr(const struct PE_(rz_bin_pe_obj_t) * bin, ut16 dllCharacteristic) {
return bin->nt_headers->optional_header.DllCharacteristics & dllCharacteristic;
}
Expand Down Expand Up @@ -703,15 +696,15 @@ static RzBinInfo *info(RzBinFile *bf) {
ret->bits = PE_(rz_bin_pe_get_bits)(bf->o->bin_obj);
ret->big_endian = PE_(rz_bin_pe_is_big_endian)(bf->o->bin_obj);
ret->dbg_info = 0;
ret->has_canary = has_canary(bf);
ret->has_canary = PE_(rz_bin_pe_has_canary)(bf->o->bin_obj);
ret->has_nx = haschr(bin, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT);
ret->has_pi = haschr(bin, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE);
ret->claimed_checksum = rz_str_newf("0x%08x", claimed_checksum);
ret->actual_checksum = rz_str_newf("0x%08x", actual_checksum);
ret->pe_overlay = pe_overlay > 0;
ret->signature = bin ? bin->is_signed : false;
Sdb *db = sdb_ns(bf->sdb, "pe", true);
sdb_bool_set(db, "canary", has_canary(bf));
sdb_bool_set(db, "canary", ret->has_canary);
sdb_bool_set(db, "highva", haschr(bin, IMAGE_DLLCHARACTERISTICS_HIGH_ENTROPY_VA));
sdb_bool_set(db, "aslr", haschr(bin, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE));
sdb_bool_set(db, "forceintegrity", haschr(bin, IMAGE_DLL_CHARACTERISTICS_FORCE_INTEGRITY));
Expand Down
Loading