Skip to content

Commit 9a812a4

Browse files
claudekalwalt
authored andcommitted
Fix native bugs: NULL deref, exit() calls, leaks, pointer truncation
- loadNFTMarker now returns FALSE on ar2ReadSurfaceSet failure instead of continuing past the error log and dereferencing a NULL surfaceSet. - exit(0) / exit(-1) on file or PAGES_MAX errors replaced with returning FALSE; in a browser these calls used to tear down the entire wasm runtime. - The PAGES_MAX bounds check moved to the top of loadNFTMarker so it fires before the surfaceSet array is written. - Removed the two stray malloc()s in setup(): imgBW was leaked on every loadNFTMarker (which overwrites the pointer with the surface set's own buffer), and the F_points_NFT malloc sized from uninitialized num_F_points_NFT was UB. Both pointers are now initialized to NULL and assigned by loadNFTMarker. - Pointer log statements use %p / (void *) instead of %d / (int) so values aren't truncated on targets where sizeof(int) < sizeof(void *). - Zero-initialized the remaining arFset members so a partial setup cannot leak indeterminate values into later reads. Closes #19
1 parent 00f171f commit 9a812a4

1 file changed

Lines changed: 25 additions & 20 deletions

File tree

emscripten/ARimageFsetDisplay.cpp

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -76,18 +76,18 @@ struct arFset {
7676
int height = 0;
7777
ARUint8 *videoFrame = NULL;
7878
ARUint8 *imgBW = NULL;
79-
int videoFrameSize;
80-
int imgBWsize;
79+
int videoFrameSize = 0;
80+
int imgBWsize = 0;
8181
AR2ImageSetT *imageSet = NULL;
82-
AR2SurfaceSetT *surfaceSet[PAGES_MAX];
83-
KpmRefDataSet *refDataSet;
84-
int width_NFT;
85-
int height_NFT;
86-
int dpi_NFT;
87-
int num_F_set_NFT; // number of Feature sets
88-
int num_F_points_NFT; // number of Feature points
89-
AR2FeaturePointsT *F_points_NFT;
90-
int surfaceSetCount = 0; // Running NFT marker id
82+
AR2SurfaceSetT *surfaceSet[PAGES_MAX] = {};
83+
KpmRefDataSet *refDataSet = NULL;
84+
int width_NFT = 0;
85+
int height_NFT = 0;
86+
int dpi_NFT = 0;
87+
int num_F_set_NFT = 0; // number of Feature sets
88+
int num_F_points_NFT = 0; // number of Feature points
89+
AR2FeaturePointsT *F_points_NFT = NULL;
90+
int surfaceSetCount = 0; // Running NFT marker id
9191
};
9292

9393
std::unordered_map<int, arFset> arFsets;
@@ -101,12 +101,18 @@ int loadNFTMarker(arFset *arc, int surfaceSetCount,
101101
const char *datasetPathname) {
102102
int i, pageNo, numIset, width, height, dpi;
103103

104+
if (surfaceSetCount >= PAGES_MAX) {
105+
ARLOGe("Cannot load more than %d NFT markers\n", PAGES_MAX);
106+
return FALSE;
107+
}
108+
104109
// Load AR2 data.
105110
ARLOGi("Reading %s.fset\n", datasetPathname);
106111

107112
if ((arc->surfaceSet[surfaceSetCount] =
108113
ar2ReadSurfaceSet(datasetPathname, "fset", NULL)) == NULL) {
109114
ARLOGe("Error reading data from %s.fset\n", datasetPathname);
115+
return FALSE;
110116
}
111117

112118
numIset = arc->surfaceSet[surfaceSetCount]->surface[0].imageSet->num;
@@ -125,7 +131,7 @@ int loadNFTMarker(arFset *arc, int surfaceSetCount,
125131
arc->imgBW =
126132
arc->surfaceSet[surfaceSetCount]->surface[0].imageSet->scale[0]->imgBW;
127133

128-
ARLOGi("printing pointer imgBW: %d\n", arc->imgBW);
134+
ARLOGi("printing pointer imgBW: %p\n", (void *)arc->imgBW);
129135

130136
ARLOGi("NFT number of ImageSet: %i\n", numIset);
131137
ARLOGi("NFT marker width: %i\n", arc->width_NFT);
@@ -143,14 +149,11 @@ int loadNFTMarker(arFset *arc, int surfaceSetCount,
143149
kpmLoadRefDataSet( datasetPathname, "fset3", &arc->refDataSet );
144150
if( arc->refDataSet == NULL ) {
145151
ARLOGe("file open error: %s.fset3\n", datasetPathname );
146-
exit(0);
152+
return FALSE;
147153
}
148154
ARLOGi(" end.\n");
149155
ARLOGi("num = %d\n", arc->refDataSet->num);
150156

151-
if (surfaceSetCount == PAGES_MAX)
152-
exit(-1);
153-
154157
ARLOGi("imgsizePointer: %d\n", arc->imgBWsize);
155158

156159
ARLOGi("Loading of NFT data complete.\n");
@@ -170,7 +173,7 @@ nftMarker readNFTMarker(int id, std::string datasetPathname) {
170173
ARLOGe("ARimageFsetDisplay(): Unable to read NFT marker.\n");
171174
return nft;
172175
} else {
173-
ARLOGi("Passing the imgBW pointer: %d\n", (int)arc->imgBW);
176+
ARLOGi("Passing the imgBW pointer: %p\n", (void *)arc->imgBW);
174177
}
175178

176179
arc->surfaceSetCount++;
@@ -208,10 +211,12 @@ int setup(int width, int height) {
208211
arFset *arc = &(arFsets[id]);
209212
arc->id = id;
210213
arc->imgBWsize = width * height * 4 * sizeof(ARUint8);
211-
arc->imgBW = (ARUint8 *)malloc(arc->imgBWsize);
212-
arc->F_points_NFT = (AR2FeaturePointsT *)malloc(arc->num_F_points_NFT * 2);
214+
// imgBW and F_points_NFT are filled in by loadNFTMarker from the loaded
215+
// surface set, so we don't allocate them here.
216+
arc->imgBW = NULL;
217+
arc->F_points_NFT = NULL;
213218

214-
ARLOGi("Allocated imgBWsize %d\n", arc->imgBWsize);
219+
ARLOGi("Reserved imgBWsize %d\n", arc->imgBWsize);
215220

216221
return arc->id;
217222
}

0 commit comments

Comments
 (0)