Skip to content

Commit b07f98b

Browse files
dguddgud
committed
wx: Fix array reading out of bounds, and potential memory leaks
Issues reported in the excellent blog post at https://pvs-studio.com/en/blog/posts/cpp/1305/
1 parent 412bff5 commit b07f98b

File tree

5 files changed

+56
-27
lines changed

5 files changed

+56
-27
lines changed

‎lib/wx/api_gen/wx_gen_nif.erl‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@ gen_method(CName, M=#method{name=N,params=Ps0,type=T,method_type=MT,id=MethodId
386386
Opts = [Opt || Opt = #param{def=Def,in=In,where=Where} <- Ps2,
387387
Def =/= none, In =/= false, Where =/= c],
388388
decode_options(Opts, Argc),
389+
389390
case gen_util:get_hook(c, M#method.pre_hook) of
390391
ignore -> skip;
391392
Pre -> w(" ~s;~n", [Pre])
@@ -540,6 +541,23 @@ decode_opt(#param{name=Name,type=Type}) ->
540541
decode_arguments(Ps0) ->
541542
lists:mapfoldl(fun decode_arg/2,0,Ps0).
542543

544+
%% Postpone alloc and memcpy until after all Badarg execeptions
545+
%% so we don't leak memory in case of later badarg
546+
copy_arguments([#param{where=Where, in=In, def=none, name=N, type=Type}|Rest])
547+
when Where =/= erl, Where =/= c, In =/= false ->
548+
case Type of
549+
#type{base = binary, by_val=copy} ->
550+
w(" ~s = (unsigned char *) malloc(~s_bin.size);\n", [N,N]),
551+
w(" memcpy(~s,~s_bin.data,~s_bin.size);\n", [N,N,N]);
552+
_ ->
553+
ignore
554+
end,
555+
copy_arguments(Rest);
556+
copy_arguments([_|Rest]) ->
557+
copy_arguments(Rest);
558+
copy_arguments([]) ->
559+
ok.
560+
543561
store_free(N) ->
544562
case get(free_args) of
545563
undefined -> put(free_args, [N]);
@@ -708,9 +726,10 @@ decode_arg(N,#type{name=Type,base=binary,mod=Mod0,by_val=Copy},Arg,Argc) ->
708726
w(" ErlNifBinary ~s_bin;~n",[N]),
709727
w(" if(!enif_inspect_binary(env, ~s, &~s_bin)) ~s;~n",[Argc, N, badarg(N)]),
710728
case Copy of
711-
copy ->
712-
w(" ~s = (unsigned char *) malloc(~s_bin.size);\n", [N,N]),
713-
w(" memcpy(~s,~s_bin.data,~s_bin.size);\n", [N,N,N]);
729+
copy -> %% postpone see copy_arguments
730+
%% w(" ~s = (unsigned char *) malloc(~s_bin.size);\n", [N,N]),
731+
%% w(" memcpy(~s,~s_bin.data,~s_bin.size);\n", [N,N,N]);
732+
ok;
714733
_ ->
715734
w(" ~s = (~s~s*) ~s_bin.data;~n", [N,Mod,Type,N])
716735
end;
@@ -795,6 +814,8 @@ call_wx(_N,{constructor,_},#type{base={class,RClass}},Ps) ->
795814
false -> RClass
796815
end,
797816

817+
copy_arguments(Ps),
818+
798819
case [P || #param{type={merged,_}}=P <- Ps] of
799820
[] ->
800821
w(" ~s * Result = new ~s(~s);~n",
@@ -836,6 +857,7 @@ call_wx(_N,{constructor,_},#type{base={class,RClass}},Ps) ->
836857
call_wx(N,{member,_},Type,Ps0) ->
837858
{Beg,End} = return_res(Type),
838859
w(" if(!This) throw wxe_badarg(\"This\");~n",[]),
860+
copy_arguments(Ps0),
839861
Ps = filter(Ps0),
840862
case [P || #param{type={merged,_}}=P <- Ps] of
841863
[] ->
@@ -856,6 +878,7 @@ call_wx(N,{member,_},Type,Ps0) ->
856878
call_wx(N,{static,Class},Type,Ps) ->
857879
{Beg,End} = return_res(Type),
858880
#class{parent=Parent} = get({class,Class}),
881+
copy_arguments(Ps),
859882
case [P || #param{type={merged,_}}=P <- Ps] of
860883
[] when Parent =:= "static" ->
861884
w(" ~s::~s(~s)~s;~n",[Beg,N,args(fun call_arg/1, ",",filter(Ps)),End]);

‎lib/wx/c_src/gen/wxe_wrapper_4.cpp‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
22
* %CopyrightBegin%
33
*
4-
* Copyright Ericsson AB 2008-2022. All Rights Reserved.
4+
* Copyright Ericsson AB 2008-2025. All Rights Reserved.
55
*
66
* Licensed under the Apache License, Version 2.0 (the "License");
77
* you may not use this file except in compliance with the License.
@@ -1291,11 +1291,11 @@ void wxImage_new_4(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
12911291
unsigned char * data;
12921292
ErlNifBinary data_bin;
12931293
if(!enif_inspect_binary(env, argv[2], &data_bin)) Badarg("data");
1294-
data = (unsigned char *) malloc(data_bin.size);
1295-
memcpy(data,data_bin.data,data_bin.size);
12961294
unsigned char * alpha;
12971295
ErlNifBinary alpha_bin;
12981296
if(!enif_inspect_binary(env, argv[3], &alpha_bin)) Badarg("alpha");
1297+
data = (unsigned char *) malloc(data_bin.size);
1298+
memcpy(data,data_bin.data,data_bin.size);
12991299
alpha = (unsigned char *) malloc(alpha_bin.size);
13001300
memcpy(alpha,alpha_bin.data,alpha_bin.size);
13011301
wxImage * Result = new EwxImage(width,height,data,alpha);
@@ -1321,11 +1321,11 @@ void wxImage_new_3_3(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
13211321
unsigned char * data;
13221322
ErlNifBinary data_bin;
13231323
if(!enif_inspect_binary(env, argv[1], &data_bin)) Badarg("data");
1324-
data = (unsigned char *) malloc(data_bin.size);
1325-
memcpy(data,data_bin.data,data_bin.size);
13261324
unsigned char * alpha;
13271325
ErlNifBinary alpha_bin;
13281326
if(!enif_inspect_binary(env, argv[2], &alpha_bin)) Badarg("alpha");
1327+
data = (unsigned char *) malloc(data_bin.size);
1328+
memcpy(data,data_bin.data,data_bin.size);
13291329
alpha = (unsigned char *) malloc(alpha_bin.size);
13301330
memcpy(alpha,alpha_bin.data,alpha_bin.size);
13311331
wxImage * Result = new EwxImage(sz,data,alpha);
@@ -1656,9 +1656,9 @@ void wxImage_Create_3_0(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
16561656
unsigned char * data;
16571657
ErlNifBinary data_bin;
16581658
if(!enif_inspect_binary(env, argv[3], &data_bin)) Badarg("data");
1659+
if(!This) throw wxe_badarg("This");
16591660
data = (unsigned char *) malloc(data_bin.size);
16601661
memcpy(data,data_bin.data,data_bin.size);
1661-
if(!This) throw wxe_badarg("This");
16621662
bool Result = This->Create(width,height,data);
16631663
wxeReturn rt = wxeReturn(memenv, Ecmd.caller, true);
16641664
rt.send( rt.make_bool(Result));
@@ -1683,9 +1683,9 @@ void wxImage_Create_2_0(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
16831683
unsigned char * data;
16841684
ErlNifBinary data_bin;
16851685
if(!enif_inspect_binary(env, argv[2], &data_bin)) Badarg("data");
1686+
if(!This) throw wxe_badarg("This");
16861687
data = (unsigned char *) malloc(data_bin.size);
16871688
memcpy(data,data_bin.data,data_bin.size);
1688-
if(!This) throw wxe_badarg("This");
16891689
bool Result = This->Create(sz,data);
16901690
wxeReturn rt = wxeReturn(memenv, Ecmd.caller, true);
16911691
rt.send( rt.make_bool(Result));
@@ -1706,14 +1706,14 @@ void wxImage_Create_4(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
17061706
unsigned char * data;
17071707
ErlNifBinary data_bin;
17081708
if(!enif_inspect_binary(env, argv[3], &data_bin)) Badarg("data");
1709-
data = (unsigned char *) malloc(data_bin.size);
1710-
memcpy(data,data_bin.data,data_bin.size);
17111709
unsigned char * alpha;
17121710
ErlNifBinary alpha_bin;
17131711
if(!enif_inspect_binary(env, argv[4], &alpha_bin)) Badarg("alpha");
1712+
if(!This) throw wxe_badarg("This");
1713+
data = (unsigned char *) malloc(data_bin.size);
1714+
memcpy(data,data_bin.data,data_bin.size);
17141715
alpha = (unsigned char *) malloc(alpha_bin.size);
17151716
memcpy(alpha,alpha_bin.data,alpha_bin.size);
1716-
if(!This) throw wxe_badarg("This");
17171717
bool Result = This->Create(width,height,data,alpha);
17181718
wxeReturn rt = wxeReturn(memenv, Ecmd.caller, true);
17191719
rt.send( rt.make_bool(Result));
@@ -1738,14 +1738,14 @@ void wxImage_Create_3_2(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
17381738
unsigned char * data;
17391739
ErlNifBinary data_bin;
17401740
if(!enif_inspect_binary(env, argv[2], &data_bin)) Badarg("data");
1741-
data = (unsigned char *) malloc(data_bin.size);
1742-
memcpy(data,data_bin.data,data_bin.size);
17431741
unsigned char * alpha;
17441742
ErlNifBinary alpha_bin;
17451743
if(!enif_inspect_binary(env, argv[3], &alpha_bin)) Badarg("alpha");
1744+
if(!This) throw wxe_badarg("This");
1745+
data = (unsigned char *) malloc(data_bin.size);
1746+
memcpy(data,data_bin.data,data_bin.size);
17461747
alpha = (unsigned char *) malloc(alpha_bin.size);
17471748
memcpy(alpha,alpha_bin.data,alpha_bin.size);
1748-
if(!This) throw wxe_badarg("This");
17491749
bool Result = This->Create(sz,data,alpha);
17501750
wxeReturn rt = wxeReturn(memenv, Ecmd.caller, true);
17511751
rt.send( rt.make_bool(Result));
@@ -2673,9 +2673,9 @@ void wxImage_SetAlpha_1(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
26732673
unsigned char * alpha;
26742674
ErlNifBinary alpha_bin;
26752675
if(!enif_inspect_binary(env, argv[1], &alpha_bin)) Badarg("alpha");
2676+
if(!This) throw wxe_badarg("This");
26762677
alpha = (unsigned char *) malloc(alpha_bin.size);
26772678
memcpy(alpha,alpha_bin.data,alpha_bin.size);
2678-
if(!This) throw wxe_badarg("This");
26792679
This->SetAlpha(alpha);
26802680

26812681
}
@@ -2708,9 +2708,9 @@ void wxImage_SetData_1(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
27082708
unsigned char * data;
27092709
ErlNifBinary data_bin;
27102710
if(!enif_inspect_binary(env, argv[1], &data_bin)) Badarg("data");
2711+
if(!This) throw wxe_badarg("This");
27112712
data = (unsigned char *) malloc(data_bin.size);
27122713
memcpy(data,data_bin.data,data_bin.size);
2713-
if(!This) throw wxe_badarg("This");
27142714
This->SetData(data);
27152715

27162716
}
@@ -2725,13 +2725,13 @@ void wxImage_SetData_3(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
27252725
unsigned char * data;
27262726
ErlNifBinary data_bin;
27272727
if(!enif_inspect_binary(env, argv[1], &data_bin)) Badarg("data");
2728-
data = (unsigned char *) malloc(data_bin.size);
2729-
memcpy(data,data_bin.data,data_bin.size);
27302728
int new_width;
27312729
if(!enif_get_int(env, argv[2], &new_width)) Badarg("new_width"); // int
27322730
int new_height;
27332731
if(!enif_get_int(env, argv[3], &new_height)) Badarg("new_height"); // int
27342732
if(!This) throw wxe_badarg("This");
2733+
data = (unsigned char *) malloc(data_bin.size);
2734+
memcpy(data,data_bin.data,data_bin.size);
27352735
This->SetData(data,new_width,new_height);
27362736

27372737
}

‎lib/wx/c_src/wxe_gl.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,19 @@ void setActiveGL(wxeMemEnv *memenv, ErlNifPid caller, wxGLCanvas *canvas, wxGLCo
7373
{
7474
ErlNifUInt64 callId = wxe_make_hash(memenv->tmp_env, &caller);
7575
wxe_glc * entry = glc[callId];
76+
7677
gl_active_index = callId;
7778
gl_active_pid = caller;
7879

7980
if(!entry) {
80-
entry = (wxe_glc *) malloc(sizeof(wxe_glc));
81-
entry->canvas = NULL;
82-
entry->context = NULL;
81+
if(canvas && context) {
82+
entry = (wxe_glc *) malloc(sizeof(wxe_glc));
83+
entry->canvas = NULL;
84+
entry->context = NULL;
85+
}
86+
else // canvas or context are NULL ignore
87+
return;
8388
}
84-
8589
if(entry->canvas == canvas && entry->context == context)
8690
return;
8791

‎lib/wx/c_src/wxe_return.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ ERL_NIF_TERM wxeReturn::make_array_objs(wxAuiPaneInfoArray& arr, WxeApp *app, co
231231
ERL_NIF_TERM head, tail;
232232
ERL_NIF_TERM class_name = enif_make_atom(env, cname);
233233
tail = enif_make_list(env, 0);
234-
for(unsigned int i = arr.GetCount() -1; i >= 0; i--) {
234+
for(ErlNifSInt64 i = arr.GetCount() -1; i >= 0; i--) {
235235
head = make_ref(app->getRef((void *) &arr.Item(i),memenv), class_name);
236236
tail = enif_make_list_cell(env, head, tail);
237237
}
@@ -243,7 +243,7 @@ ERL_NIF_TERM wxeReturn::make_array_objs(wxArrayTreeItemIds& arr)
243243
{
244244
ERL_NIF_TERM head, tail;
245245
tail = enif_make_list(env, 0);
246-
for(unsigned int i = arr.GetCount() -1; i >= 0; i--) {
246+
for(ErlNifSInt64 i = arr.GetCount() -1; i >= 0; i--) {
247247
head = make((wxUIntPtr *) arr[i].m_pItem);
248248
tail = enif_make_list_cell(env, head, tail);
249249
}
@@ -255,7 +255,7 @@ ERL_NIF_TERM wxeReturn::make_array_objs(wxGridCellCoordsArray& arr)
255255
{
256256
ERL_NIF_TERM head, tail;
257257
tail = enif_make_list(env, 0);
258-
for(unsigned int i = arr.GetCount() -1; i >= 0; i--) {
258+
for(ErlNifSInt64 i = arr.GetCount() -1; i >= 0; i--) {
259259
head = make(arr[i]);
260260
tail = enif_make_list_cell(env, head, tail);
261261
}

‎lib/wx/test/wx_class_SUITE.erl‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,8 @@ treeCtrl(Config) ->
148148
?m({0, _}, wxTreeCtrl:hitTest(Tree, {X0+W0+W0, Y0+H0+4*H0})),
149149
?m(false, wxTreeCtrl:isTreeItemIdOk(0)),
150150

151+
{0, []} = wxTreeCtrl:getSelections(Tree),
152+
151153
wxFrame:connect(Tree, command_tree_item_expanded),
152154
wxFrame:connect(Tree, command_tree_item_collapsed),
153155
wxFrame:connect(Frame, close_window),

0 commit comments

Comments
 (0)