Missing Free Call for duress_hash in is_valid_duress_file
zakuArbor opened this issue · 2 comments
zakuArbor commented
Line 141 in 04e607f
I was skimming the code and noticed you forgot to free duress_hash
in the function is_valid_duress_file
since sha_256_sum
allocates memory in the heap, it should be freed before calling return
zakuArbor commented
Confirmed using asan:
$ sudo pam_test $USER
Credentials accepted.
Password:
AddressSanitizer:DEADLYSIGNAL
=================================================================
==39682==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000000000 bp 0x7ffcd3d02e30 sp 0x7ffcd3d025b8 T0)
==39682==Hint: pc points to the zero page.
==39682==The signal is caused by a READ memory access.
==39682==Hint: address points to the zero page.
#0 0x0 (<unknown module>)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (<unknown module>)
==39682==ABORTING
To replicate:
$ git diff Makefile
diff --git a/Makefile b/Makefile
index 6065001..445900e 100644
--- a/Makefile
+++ b/Makefile
@@ -47,18 +47,18 @@ install: $(BIN_DIR)/pam_duress.so $(BIN_DIR)/duress_sign $(BIN_DIR)/pam_test
$(OBJS): $(OBJ_DIR)/%.o : $(SRC_DIR)/%.c
mkdir -p $(OBJ_DIR)
- $(CC) -o $@ $(CFLAGS) -c $< $(LDINCLUDE)
+ $(CC) -g -fsanitize=address -o $@ $(CFLAGS) -c $< $(LDINCLUDE)
$(BIN_DIR)/duress_sign: $(OBJ_DIR)/duress_sign.o $(OBJ_DIR)/util.o
mkdir -p $(BIN_DIR)
- $(CC) -o $@ $^ $(LDLIB)
+ $(CC) -fsanitize=address -g -o $@ $^ $(LDLIB)
$(BIN_DIR)/pam_duress.so: $(OBJ_DIR)/duress.o $(OBJ_DIR)/util.o
ld $(LDFLAGS) -o $@ $^ $(LDLIB)
$(BIN_DIR)/pam_test: $(SRC_DIR)/pam_test.c
mkdir -p $(BIN_DIR)
- $(CC) -o $@ $^ $(LDLIB)
+ $(CC) -g -fsanitize=address -o $@ $^ $(LDLIB)
.PHONY: uninstall
uninstall:
zakuArbor commented
Using valgrind instead:
zaku@zaku:~/Documents/pam-duress$ valgrind pam_test $USER 2>&1 | tail -n 20
==40403== Warning: invalid file descriptor 1028 in syscall close()
==40403== Warning: invalid file descriptor 1029 in syscall close()
Credentials accepted.
Account is valid.
Authenticated
==40398==
==40398== HEAP SUMMARY:
==40398== in use at exit: 5,140 bytes in 17 blocks
==40398== total heap usage: 5,716 allocs, 5,699 frees, 1,261,860 bytes allocated
==40398==
==40398== LEAK SUMMARY:
==40398== definitely lost: 51 bytes in 2 blocks
==40398== indirectly lost: 0 bytes in 0 blocks
==40398== possibly lost: 0 bytes in 0 blocks
==40398== still reachable: 5,089 bytes in 15 blocks
==40398== suppressed: 0 bytes in 0 blocks
==40398== Rerun with --leak-check=full to see details of leaked memory
==40398==
==40398== For lists of detected and suppressed errors, rerun with: -s
==40398== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
With fix implemented:
zaku@zaku:~/Documents/pam-duress$ valgrind pam_test $USER 2>&1 | tail -n 20
==40892== Warning: invalid file descriptor 1028 in syscall close()
==40892== Warning: invalid file descriptor 1029 in syscall close()
Credentials accepted.
Account is valid.
Authenticated
==40887==
==40887== HEAP SUMMARY:
==40887== in use at exit: 5,108 bytes in 16 blocks
==40887== total heap usage: 5,716 allocs, 5,700 frees, 1,261,860 bytes allocated
==40887==
==40887== LEAK SUMMARY:
==40887== definitely lost: 19 bytes in 1 blocks
==40887== indirectly lost: 0 bytes in 0 blocks
==40887== possibly lost: 0 bytes in 0 blocks
==40887== still reachable: 5,089 bytes in 15 blocks
==40887== suppressed: 0 bytes in 0 blocks
==40887== Rerun with --leak-check=full to see details of leaked memory
==40887==
==40887== For lists of detected and suppressed errors, rerun with: -s
==40887== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Seems like there are more memory leaks.