From c5b6bb0d4bd8006b89f308021e0bc5450769b590 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 25 Sep 2016 16:43:54 +0100 Subject: [PATCH 1/3] snd_wavelet: avoid undefined pointer below array bounds Given an array b[] of length n, pointers to &b[0]..&b[n] are defined (where only &b[0]..&b[n-1] can be validly dereferenced). &b[-1], or equivalently b-1, is not something we can use in valid Standard C. gcc 6 diagnoses this as: code/client/snd_wavelet.c:33:9: warning: array subscript is below array bounds [-Warray-bounds] and might take this undefined behaviour as permission to emit "more efficient" object code that is not what the author expected, for example nothing at all. Use a macro to fake a 1-based array instead. --- code/client/snd_wavelet.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/code/client/snd_wavelet.c b/code/client/snd_wavelet.c index 3d8f5c7b..e051da29 100644 --- a/code/client/snd_wavelet.c +++ b/code/client/snd_wavelet.c @@ -30,7 +30,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA void daub4(float b[], unsigned long n, int isign) { float wksp[4097] = { 0.0f }; - float *a=b-1; // numerical recipies so a[1] = b[0] +#define a(x) b[(x)-1] // numerical recipies so a[1] = b[0] unsigned long nh,nh1,i,j; @@ -39,22 +39,23 @@ void daub4(float b[], unsigned long n, int isign) nh1=(nh=n >> 1)+1; if (isign >= 0) { for (i=1,j=1;j<=n-3;j+=2,i++) { - wksp[i] = C0*a[j]+C1*a[j+1]+C2*a[j+2]+C3*a[j+3]; - wksp[i+nh] = C3*a[j]-C2*a[j+1]+C1*a[j+2]-C0*a[j+3]; + wksp[i] = C0*a(j)+C1*a(j+1)+C2*a(j+2)+C3*a(j+3); + wksp[i+nh] = C3*a(j)-C2*a(j+1)+C1*a(j+2)-C0*a(j+3); } - wksp[i ] = C0*a[n-1]+C1*a[n]+C2*a[1]+C3*a[2]; - wksp[i+nh] = C3*a[n-1]-C2*a[n]+C1*a[1]-C0*a[2]; + wksp[i ] = C0*a(n-1)+C1*a(n)+C2*a(1)+C3*a(2); + wksp[i+nh] = C3*a(n-1)-C2*a(n)+C1*a(1)-C0*a(2); } else { - wksp[1] = C2*a[nh]+C1*a[n]+C0*a[1]+C3*a[nh1]; - wksp[2] = C3*a[nh]-C0*a[n]+C1*a[1]-C2*a[nh1]; + wksp[1] = C2*a(nh)+C1*a(n)+C0*a(1)+C3*a(nh1); + wksp[2] = C3*a(nh)-C0*a(n)+C1*a(1)-C2*a(nh1); for (i=1,j=3;i Date: Sun, 25 Sep 2016 16:57:52 +0100 Subject: [PATCH 2/3] UI_BuildFindPlayerList: avoid array underflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function is used in the Team Arena menus I don't think it's actually possible to reach this line with foundPlayerServerNames < 1, because by the time we get here we have set it to 1 + the actual number of servers; but if we did, it would clearly underflow into foundPlayerServerNames[-1], which would be undefined behaviour. gcc 6 diagnoses this with a warning: code/ui/ui_main.c: In function ‘UI_BuildFindPlayerList’: code/ui/ui_main.c:4138:16: warning: array subscript is below array bounds [-Warray-bounds] Also correct the sizeof() invocation to make it more obviously correct (in fact the buffers for names and addresses happen to both be of size MAX_ADDRESSLENGTH, so it was fine, but it's good to be obvious). --- code/ui/ui_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/ui/ui_main.c b/code/ui/ui_main.c index a1539f33..044c00ff 100644 --- a/code/ui/ui_main.c +++ b/code/ui/ui_main.c @@ -4135,7 +4135,7 @@ static void UI_BuildFindPlayerList(qboolean force) { else { // add a line that shows the number of servers found if (!uiInfo.numFoundPlayerServers) { - Com_sprintf(uiInfo.foundPlayerServerNames[uiInfo.numFoundPlayerServers-1], sizeof(uiInfo.foundPlayerServerAddresses[0]), "no servers found"); + Com_sprintf(uiInfo.foundPlayerServerNames[0], sizeof(uiInfo.foundPlayerServerNames[0]), "no servers found"); } else { Com_sprintf(uiInfo.foundPlayerServerNames[uiInfo.numFoundPlayerServers-1], sizeof(uiInfo.foundPlayerServerAddresses[0]), From f1a133acf10cab123518e39478875f22b6e91f85 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 25 Sep 2016 16:59:12 +0100 Subject: [PATCH 3/3] UI_BuildFindPlayerList: make a sizeof() more obviously correct As with the other branch of the if/else, each element of foundPlayerServerNames is in fact the same size as each element of foundPlayerServerAddresses, so it was fine; but it's better to make it obvious that we are using the right array sizes. --- code/ui/ui_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/ui/ui_main.c b/code/ui/ui_main.c index 044c00ff..e9e36ea9 100644 --- a/code/ui/ui_main.c +++ b/code/ui/ui_main.c @@ -4138,7 +4138,7 @@ static void UI_BuildFindPlayerList(qboolean force) { Com_sprintf(uiInfo.foundPlayerServerNames[0], sizeof(uiInfo.foundPlayerServerNames[0]), "no servers found"); } else { - Com_sprintf(uiInfo.foundPlayerServerNames[uiInfo.numFoundPlayerServers-1], sizeof(uiInfo.foundPlayerServerAddresses[0]), + Com_sprintf(uiInfo.foundPlayerServerNames[uiInfo.numFoundPlayerServers-1], sizeof(uiInfo.foundPlayerServerNames[0]), "%d server%s found with player %s", uiInfo.numFoundPlayerServers-1, uiInfo.numFoundPlayerServers == 2 ? "":"s", uiInfo.findPlayerName); }