Skip to content

Commit aa16380

Browse files
dguddgud
committed
Merge branch 'dgud/wx/bugs/OTP-19843' into dgud/wx/bugs/28/OTP-19843
* dgud/wx/bugs/OTP-19843: wx: Fix array reading out of bounds, and potential memory leaks
2 parents a2e0f50 + b07f98b commit aa16380

File tree

5 files changed

+56
-26
lines changed

5 files changed

+56
-26
lines changed

‎lib/wx/api_gen/wx_gen_nif.erl‎

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

546+
%% Postpone alloc and memcpy until after all Badarg execeptions
547+
%% so we don't leak memory in case of later badarg
548+
copy_arguments([#param{where=Where, in=In, def=none, name=N, type=Type}|Rest])
549+
when Where =/= erl, Where =/= c, In =/= false ->
550+
case Type of
551+
#type{base = binary, by_val=copy} ->
552+
w(" ~s = (unsigned char *) malloc(~s_bin.size);\n", [N,N]),
553+
w(" memcpy(~s,~s_bin.data,~s_bin.size);\n", [N,N,N]);
554+
_ ->
555+
ignore
556+
end,
557+
copy_arguments(Rest);
558+
copy_arguments([_|Rest]) ->
559+
copy_arguments(Rest);
560+
copy_arguments([]) ->
561+
ok.
562+
545563
store_free(N) ->
546564
case get(free_args) of
547565
undefined -> put(free_args, [N]);
@@ -710,9 +728,10 @@ decode_arg(N,#type{name=Type,base=binary,mod=Mod0,by_val=Copy},Arg,Argc) ->
710728
w(" ErlNifBinary ~s_bin;~n",[N]),
711729
w(" if(!enif_inspect_binary(env, ~s, &~s_bin)) ~s;~n",[Argc, N, badarg(N)]),
712730
case Copy of
713-
copy ->
714-
w(" ~s = (unsigned char *) malloc(~s_bin.size);\n", [N,N]),
715-
w(" memcpy(~s,~s_bin.data,~s_bin.size);\n", [N,N,N]);
731+
copy -> %% postpone see copy_arguments
732+
%% w(" ~s = (unsigned char *) malloc(~s_bin.size);\n", [N,N]),
733+
%% w(" memcpy(~s,~s_bin.data,~s_bin.size);\n", [N,N,N]);
734+
ok;
716735
_ ->
717736
w(" ~s = (~s~s*) ~s_bin.data;~n", [N,Mod,Type,N])
718737
end;
@@ -797,6 +816,8 @@ call_wx(_N,{constructor,_},#type{base={class,RClass}},Ps) ->
797816
false -> RClass
798817
end,
799818

819+
copy_arguments(Ps),
820+
800821
case [P || #param{type={merged,_}}=P <- Ps] of
801822
[] ->
802823
w(" ~s * Result = new ~s(~s);~n",
@@ -838,6 +859,7 @@ call_wx(_N,{constructor,_},#type{base={class,RClass}},Ps) ->
838859
call_wx(N,{member,_},Type,Ps0) ->
839860
{Beg,End} = return_res(Type),
840861
w(" if(!This) throw wxe_badarg(\"This\");~n",[]),
862+
copy_arguments(Ps0),
841863
Ps = filter(Ps0),
842864
case [P || #param{type={merged,_}}=P <- Ps] of
843865
[] ->
@@ -858,6 +880,7 @@ call_wx(N,{member,_},Type,Ps0) ->
858880
call_wx(N,{static,Class},Type,Ps) ->
859881
{Beg,End} = return_res(Type),
860882
#class{parent=Parent} = get({class,Class}),
883+
copy_arguments(Ps),
861884
case [P || #param{type={merged,_}}=P <- Ps] of
862885
[] when Parent =:= "static" ->
863886
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: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1293,11 +1293,11 @@ void wxImage_new_4(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
12931293
unsigned char * data;
12941294
ErlNifBinary data_bin;
12951295
if(!enif_inspect_binary(env, argv[2], &data_bin)) Badarg("data");
1296-
data = (unsigned char *) malloc(data_bin.size);
1297-
memcpy(data,data_bin.data,data_bin.size);
12981296
unsigned char * alpha;
12991297
ErlNifBinary alpha_bin;
13001298
if(!enif_inspect_binary(env, argv[3], &alpha_bin)) Badarg("alpha");
1299+
data = (unsigned char *) malloc(data_bin.size);
1300+
memcpy(data,data_bin.data,data_bin.size);
13011301
alpha = (unsigned char *) malloc(alpha_bin.size);
13021302
memcpy(alpha,alpha_bin.data,alpha_bin.size);
13031303
wxImage * Result = new EwxImage(width,height,data,alpha);
@@ -1323,11 +1323,11 @@ void wxImage_new_3_3(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
13231323
unsigned char * data;
13241324
ErlNifBinary data_bin;
13251325
if(!enif_inspect_binary(env, argv[1], &data_bin)) Badarg("data");
1326-
data = (unsigned char *) malloc(data_bin.size);
1327-
memcpy(data,data_bin.data,data_bin.size);
13281326
unsigned char * alpha;
13291327
ErlNifBinary alpha_bin;
13301328
if(!enif_inspect_binary(env, argv[2], &alpha_bin)) Badarg("alpha");
1329+
data = (unsigned char *) malloc(data_bin.size);
1330+
memcpy(data,data_bin.data,data_bin.size);
13311331
alpha = (unsigned char *) malloc(alpha_bin.size);
13321332
memcpy(alpha,alpha_bin.data,alpha_bin.size);
13331333
wxImage * Result = new EwxImage(sz,data,alpha);
@@ -1658,9 +1658,9 @@ void wxImage_Create_3_0(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
16581658
unsigned char * data;
16591659
ErlNifBinary data_bin;
16601660
if(!enif_inspect_binary(env, argv[3], &data_bin)) Badarg("data");
1661+
if(!This) throw wxe_badarg("This");
16611662
data = (unsigned char *) malloc(data_bin.size);
16621663
memcpy(data,data_bin.data,data_bin.size);
1663-
if(!This) throw wxe_badarg("This");
16641664
bool Result = This->Create(width,height,data);
16651665
wxeReturn rt = wxeReturn(memenv, Ecmd.caller, true);
16661666
rt.send( rt.make_bool(Result));
@@ -1685,9 +1685,9 @@ void wxImage_Create_2_0(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
16851685
unsigned char * data;
16861686
ErlNifBinary data_bin;
16871687
if(!enif_inspect_binary(env, argv[2], &data_bin)) Badarg("data");
1688+
if(!This) throw wxe_badarg("This");
16881689
data = (unsigned char *) malloc(data_bin.size);
16891690
memcpy(data,data_bin.data,data_bin.size);
1690-
if(!This) throw wxe_badarg("This");
16911691
bool Result = This->Create(sz,data);
16921692
wxeReturn rt = wxeReturn(memenv, Ecmd.caller, true);
16931693
rt.send( rt.make_bool(Result));
@@ -1708,14 +1708,14 @@ void wxImage_Create_4(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
17081708
unsigned char * data;
17091709
ErlNifBinary data_bin;
17101710
if(!enif_inspect_binary(env, argv[3], &data_bin)) Badarg("data");
1711-
data = (unsigned char *) malloc(data_bin.size);
1712-
memcpy(data,data_bin.data,data_bin.size);
17131711
unsigned char * alpha;
17141712
ErlNifBinary alpha_bin;
17151713
if(!enif_inspect_binary(env, argv[4], &alpha_bin)) Badarg("alpha");
1714+
if(!This) throw wxe_badarg("This");
1715+
data = (unsigned char *) malloc(data_bin.size);
1716+
memcpy(data,data_bin.data,data_bin.size);
17161717
alpha = (unsigned char *) malloc(alpha_bin.size);
17171718
memcpy(alpha,alpha_bin.data,alpha_bin.size);
1718-
if(!This) throw wxe_badarg("This");
17191719
bool Result = This->Create(width,height,data,alpha);
17201720
wxeReturn rt = wxeReturn(memenv, Ecmd.caller, true);
17211721
rt.send( rt.make_bool(Result));
@@ -1740,14 +1740,14 @@ void wxImage_Create_3_2(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
17401740
unsigned char * data;
17411741
ErlNifBinary data_bin;
17421742
if(!enif_inspect_binary(env, argv[2], &data_bin)) Badarg("data");
1743-
data = (unsigned char *) malloc(data_bin.size);
1744-
memcpy(data,data_bin.data,data_bin.size);
17451743
unsigned char * alpha;
17461744
ErlNifBinary alpha_bin;
17471745
if(!enif_inspect_binary(env, argv[3], &alpha_bin)) Badarg("alpha");
1746+
if(!This) throw wxe_badarg("This");
1747+
data = (unsigned char *) malloc(data_bin.size);
1748+
memcpy(data,data_bin.data,data_bin.size);
17481749
alpha = (unsigned char *) malloc(alpha_bin.size);
17491750
memcpy(alpha,alpha_bin.data,alpha_bin.size);
1750-
if(!This) throw wxe_badarg("This");
17511751
bool Result = This->Create(sz,data,alpha);
17521752
wxeReturn rt = wxeReturn(memenv, Ecmd.caller, true);
17531753
rt.send( rt.make_bool(Result));
@@ -2675,9 +2675,9 @@ void wxImage_SetAlpha_1(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
26752675
unsigned char * alpha;
26762676
ErlNifBinary alpha_bin;
26772677
if(!enif_inspect_binary(env, argv[1], &alpha_bin)) Badarg("alpha");
2678+
if(!This) throw wxe_badarg("This");
26782679
alpha = (unsigned char *) malloc(alpha_bin.size);
26792680
memcpy(alpha,alpha_bin.data,alpha_bin.size);
2680-
if(!This) throw wxe_badarg("This");
26812681
This->SetAlpha(alpha);
26822682

26832683
}
@@ -2710,9 +2710,9 @@ void wxImage_SetData_1(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
27102710
unsigned char * data;
27112711
ErlNifBinary data_bin;
27122712
if(!enif_inspect_binary(env, argv[1], &data_bin)) Badarg("data");
2713+
if(!This) throw wxe_badarg("This");
27132714
data = (unsigned char *) malloc(data_bin.size);
27142715
memcpy(data,data_bin.data,data_bin.size);
2715-
if(!This) throw wxe_badarg("This");
27162716
This->SetData(data);
27172717

27182718
}
@@ -2727,13 +2727,13 @@ void wxImage_SetData_3(WxeApp *app, wxeMemEnv *memenv, wxeCommand& Ecmd)
27272727
unsigned char * data;
27282728
ErlNifBinary data_bin;
27292729
if(!enif_inspect_binary(env, argv[1], &data_bin)) Badarg("data");
2730-
data = (unsigned char *) malloc(data_bin.size);
2731-
memcpy(data,data_bin.data,data_bin.size);
27322730
int new_width;
27332731
if(!enif_get_int(env, argv[2], &new_width)) Badarg("new_width"); // int
27342732
int new_height;
27352733
if(!enif_get_int(env, argv[3], &new_height)) Badarg("new_height"); // int
27362734
if(!This) throw wxe_badarg("This");
2735+
data = (unsigned char *) malloc(data_bin.size);
2736+
memcpy(data,data_bin.data,data_bin.size);
27372737
This->SetData(data,new_width,new_height);
27382738

27392739
}

‎lib/wx/c_src/wxe_gl.cpp‎

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

8182
if(!entry) {
82-
entry = (wxe_glc *) malloc(sizeof(wxe_glc));
83-
entry->canvas = NULL;
84-
entry->context = NULL;
83+
if(canvas && context) {
84+
entry = (wxe_glc *) malloc(sizeof(wxe_glc));
85+
entry->canvas = NULL;
86+
entry->context = NULL;
87+
}
88+
else // canvas or context are NULL ignore
89+
return;
8590
}
86-
8791
if(entry->canvas == canvas && entry->context == context)
8892
return;
8993

‎lib/wx/c_src/wxe_return.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ ERL_NIF_TERM wxeReturn::make_array_objs(wxAuiPaneInfoArray& arr, WxeApp *app, co
233233
ERL_NIF_TERM head, tail;
234234
ERL_NIF_TERM class_name = enif_make_atom(env, cname);
235235
tail = enif_make_list(env, 0);
236-
for(unsigned int i = arr.GetCount() -1; i >= 0; i--) {
236+
for(ErlNifSInt64 i = arr.GetCount() -1; i >= 0; i--) {
237237
head = make_ref(app->getRef((void *) &arr.Item(i),memenv), class_name);
238238
tail = enif_make_list_cell(env, head, tail);
239239
}
@@ -245,7 +245,7 @@ ERL_NIF_TERM wxeReturn::make_array_objs(wxArrayTreeItemIds& arr)
245245
{
246246
ERL_NIF_TERM head, tail;
247247
tail = enif_make_list(env, 0);
248-
for(unsigned int i = arr.GetCount() -1; i >= 0; i--) {
248+
for(ErlNifSInt64 i = arr.GetCount() -1; i >= 0; i--) {
249249
head = make((wxUIntPtr *) arr[i].m_pItem);
250250
tail = enif_make_list_cell(env, head, tail);
251251
}
@@ -257,7 +257,7 @@ ERL_NIF_TERM wxeReturn::make_array_objs(wxGridCellCoordsArray& arr)
257257
{
258258
ERL_NIF_TERM head, tail;
259259
tail = enif_make_list(env, 0);
260-
for(unsigned int i = arr.GetCount() -1; i >= 0; i--) {
260+
for(ErlNifSInt64 i = arr.GetCount() -1; i >= 0; i--) {
261261
head = make(arr[i]);
262262
tail = enif_make_list_cell(env, head, tail);
263263
}

‎lib/wx/test/wx_class_SUITE.erl‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,9 @@ treeCtrl(Config) ->
150150
?m({0, _}, wxTreeCtrl:hitTest(Tree, {X0+W0+W0, Y0+H0+4*H0})),
151151
?m(false, wxTreeCtrl:isTreeItemIdOk(0)),
152152

153+
{N, Sels} = wxTreeCtrl:getSelections(Tree),
154+
N = length(Sels),
155+
153156
wxFrame:connect(Tree, command_tree_item_expanded),
154157
wxFrame:connect(Tree, command_tree_item_collapsed),
155158
wxFrame:connect(Frame, close_window),

0 commit comments

Comments
 (0)