Hi Hugh,
As you might remember, some years ago I packages SDL Asylum for Debian.
Yesterday I received a bug report that the game crashes at the end of the
second level. I recompiled the game with "-fsanitize=address,undefined"
which discovered a stray pointer in the collision detection code, seemingly
always involving the alien2 function. This seems a likely cause for random
crashes... A typical backtrace looks like this:
==29192==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x7f1eceab9647 at pc 0x40f2d9 bp 0x7fffa1788110 sp 0x7fffa1788108
READ of size 1 at 0x7f1eceab9647 thread T0
#0 0x40f2d8 in albcheck(alent*)
/home/pdewacht/asylum/asylum-0.3.2/alien.c:1554
#1 0x41a26b in alien2(alent*)
/home/pdewacht/asylum/asylum-0.3.2/alien.c:353
#2 0x4215df in moval() /home/pdewacht/asylum/asylum-0.3.2/alien.c:102
#3 0x4230ad in game() /home/pdewacht/asylum/asylum-0.3.2/asylum.c:168
#4 0x4235c4 in init() /home/pdewacht/asylum/asylum-0.3.2/asylum.c:89
#5 0x402d83 in main /home/pdewacht/asylum/asylum-0.3.2/asylum.c:488
#6 0x7f1edf2a0b44 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x21b44)
#7 0x402f24 (/home/pdewacht/asylum/asylum-0.3.2/asylum+0x402f24)
0x7f1eceab9647 is located 441 bytes to the left of 147456-byte region
[0x7f1eceab9800,0x7f1eceadd800)
allocated by thread T0 here:
#0 0x7f1ee15a674f in malloc
(/usr/lib/x86_64-linux-gnu/libasan.so.1+0x5474f)
#1 0x428e32 in loadhammered(char**, char*, char*)
/home/pdewacht/asylum/asylum-0.3.2/file.c:167
Both the original submitter and I found some other problems, patches for
which are attached. They're probably not related to the crash the submitter
experienced though.
Best regards,
Peter De Wachter
From f7ef4d3c285d7946f577469e9e0b93da658f0b75 Mon Sep 17 00:00:00 2001
From: Peter De Wachter <pdewa...@gmail.com>
Date: Tue, 12 May 2015 23:27:34 +0200
Subject: [PATCH 1/5] wipealtab(): bounds check
---
alien.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/alien.c b/alien.c
index 90c2a4e..ac9ff53 100644
--- a/alien.c
+++ b/alien.c
@@ -1689,7 +1689,7 @@ void wipealtab()
//aladr = alofs;
alent* r10 = aladr;
alctr = 0;
- for (int r9 = _alno; r9 >= 0; r9--)
+ for (int r9 = _alno; r9 > 0; r9--)
{
l9:
r10->type = 0;
--
2.1.4
From eea77829cc17817abd5b218b3e1d969cec619a32 Mon Sep 17 00:00:00 2001
From: Peter De Wachter <pdewa...@gmail.com>
Date: Tue, 12 May 2015 23:33:00 +0200
Subject: [PATCH 2/5] swi_osfile(): fix EOF handling
Old code wrote an extra -1 byte, which caused a buffer overflow in
loadvitalfile().
---
file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/file.c b/file.c
index 48e1346..deda6fe 100644
--- a/file.c
+++ b/file.c
@@ -343,7 +343,7 @@ int swi_osfile(int op, const char* name, char* start, char* end)
case 14: // load file
f = fopen(name, "rb");
if (f == NULL) return -1;
- for (char* i = start; !feof(f); i++) *i = fgetc(f);
+ while ((x = fgetc(f)) != EOF) *start++ = x;
fclose(f);
return 0;
}
--
2.1.4
From 59c1b6eb856fb50157750aefe1c6f01b92c0ee10 Mon Sep 17 00:00:00 2001
From: Peter De Wachter <pdewa...@gmail.com>
Date: Tue, 12 May 2015 23:29:26 +0200
Subject: [PATCH 3/5] loadconfig(): fix scanf buffer overflow
scanf "%12s" needs a 13 byte buffer, as it will write up to 12
characters and a nul byte.
---
asylum.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/asylum.c b/asylum.c
index 8fe9a5b..0caf0fb 100644
--- a/asylum.c
+++ b/asylum.c
@@ -665,7 +665,7 @@ char idpermitstring[] = "You are now permitted to play the ID!!!\n";
void loadconfig()
{
- char keyword[12];
+ char keyword[13];
FILE* r0 = find_config(0x40); // read access
if (r0 != NULL)
--
2.1.4
From ca2b8974bd5108879850e4b916f93a845e51936f Mon Sep 17 00:00:00 2001
From: Peter De Wachter <pdewa...@gmail.com>
Date: Tue, 12 May 2015 23:38:19 +0200
Subject: [PATCH 4/5] swi_blitz_hammerop(): missing fclose()
---
file.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/file.c b/file.c
index deda6fe..d3ece9a 100644
--- a/file.c
+++ b/file.c
@@ -362,7 +362,10 @@ int swi_blitz_hammerop(int op, char* name, char* path, char* space)
fclose(f); return op;
} // file is not Hammered
- if (op == 0) return 0x24000; // hack: should return length
+ if (op == 0)
+ {
+ fclose(f); return 0x24000; // hack: should return length
+ }
char a[524288];
int p = 0;
char c;
--
2.1.4
From da88faa49b6f404a3e2adc0d2e75b1404be28cff Mon Sep 17 00:00:00 2001
From: Peter De Wachter <pdewa...@gmail.com>
Date: Tue, 12 May 2015 23:42:02 +0200
Subject: [PATCH 5/5] dropprivs(): add error checking
---
file.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/file.c b/file.c
index d3ece9a..577795d 100644
--- a/file.c
+++ b/file.c
@@ -83,8 +83,16 @@ FILE* find_config(int op)
void dropprivs()
{
#ifndef _WIN32
- setregid(getgid(), getgid());
- setreuid(getuid(), getuid());
+ if (setregid(getgid(), getgid()) != 0)
+ {
+ fprintf(stderr, "setregid failed\n");
+ exit(1);
+ }
+ if (setreuid(getuid(), getuid()) != 0)
+ {
+ fprintf(stderr, "setreuid failed\n");
+ exit(1);
+ }
#endif
}
--
2.1.4
