Skip to content

Commit

Permalink
[xlsb] avoid hard to spot crash with unhandled array formulas (#788)
Browse files Browse the repository at this point in the history
  • Loading branch information
@JanMarvin
JanMarvin committed Sep 3, 2023
1 parent c0b3085 commit 0471257
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 25 deletions.
50 changes: 31 additions & 19 deletions src/xlsb.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1594,9 +1594,9 @@ int workbook_bin(std::string filePath, std::string outPath, bool debug) {
case BrtName:
{
if (debug) Rcpp::Rcout << "<BrtName>" << std::endl;
// bin.seekg(size, bin.cur);
size_t end_pos = bin.tellg();
end_pos += size;

size_t end_pos = (size_t)bin.tellg() + (size_t)size;
if (debug) Rcpp::Rcout << "BrtName endpos: "<< end_pos << std::endl;

uint8_t chKey = 0;
Expand Down Expand Up @@ -1645,31 +1645,43 @@ int workbook_bin(std::string filePath, std::string outPath, bool debug) {
// XLNameWideString: XLWideString <= 255 characters
std::string name = XLWideString(bin, swapit);

// if ((size_t)bin.tellg() < end_pos) {
// Rcpp::Rcout << "repositioning" << std::endl;
// Rcpp::Rcout << end_pos << std::endl;
// bin.seekg(end_pos, bin.beg);
// }

std::string fml = "", comment = "";

int sharedFormula = false;
std::string fml = CellParsedFormula(bin, swapit, debug, 0, 0, sharedFormula);
fml = CellParsedFormula(bin, swapit, debug, 0, 0, sharedFormula);

std::string comment = XLNullableWideString(bin, swapit);
comment = XLNullableWideString(bin, swapit);

if (debug)
Rcpp::Rcout << "definedName: " << name << ": " << comment << std::endl;

if (fields->fOB ||fields->fFunc || fields->fProc) {
bin.seekg(end_pos, bin.beg);
if (fields->fProc && fml.compare("") != 0) {

/* -- something is wrong. error with some nhs macro xlsb file -- */
// // must be NULL
// Rcpp::Rcout << 1 << std::endl;
// std::string unusedstring1 = XLNullableWideString(bin, swapit);
//
// // must be < 32768 characters
// Rcpp::Rcout << 2 << std::endl;
// std::string description = XLNullableWideString(bin, swapit);
// Rcpp::Rcout << 3 << std::endl;
// std::string helpTopic = XLNullableWideString(bin, swapit);
//
// // must be NULL
// Rcpp::Rcout << 4 << std::endl;
// std::string unusedstring2 = XLNullableWideString(bin, swapit);
// must be NULL
if (debug) Rcpp::Rcout << 1 << std::endl;
std::string unusedstring1 = XLNullableWideString(bin, swapit);

// must be < 32768 characters
if (debug) Rcpp::Rcout << 2 << std::endl;
std::string description = XLNullableWideString(bin, swapit);
if (debug) Rcpp::Rcout << 3 << std::endl;
std::string helpTopic = XLNullableWideString(bin, swapit);

// must be NULL
if (debug) Rcpp::Rcout << 4 << std::endl;
std::string unusedstring2 = XLNullableWideString(bin, swapit);
}

if ((size_t)bin.tellg() != end_pos) {
Rprintf("%d: %d", (size_t)bin.tellg(), end_pos);
Rcpp::stop("repositioning");
}

if (fields->fBuiltin) {
Expand Down
64 changes: 58 additions & 6 deletions src/xlsb_funs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,7 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
if (debug) Rcpp::Rcout << "CellParsedFormula: " << sas.tellg() << std::endl;

cce = readbin(cce, sas, swapit);
if (cce > 16385) Rcpp::stop("wrong cce size");
if (cce >= 16385) Rcpp::stop("wrong cce size");
if (debug) Rcpp::Rcout << "cce: " << cce << std::endl;
size_t pos = sas.tellg();
// sas.seekg(cce, sas.cur);
Expand Down Expand Up @@ -1071,6 +1071,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgAttrIfError:
case PtgAttrGoTo:
{
if (debug) Rcpp::Rcout << "PtgAttrIf" <<std::endl;

uint16_t offset = 0;

offset = readbin(offset, sas, swapit);
Expand All @@ -1080,6 +1082,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co

case PtgAttrChoose:
{
if (debug) Rcpp::Rcout << "PtgAttrChoose" <<std::endl;

uint16_t cOffset = 0;
uint32_t rgOffset0 = 0, rgOffset1 = 0;

Expand Down Expand Up @@ -1329,6 +1333,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgArray2:
case PtgArray3:
{
if (debug) Rcpp::Rcout << "PtgArray" <<std::endl;

RgbExtra typ = PtgExtraArray;
ptgextra.push_back(typ);

Expand Down Expand Up @@ -1356,6 +1362,7 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgRef2:
case PtgRef3:
{
if (debug) Rcpp::Rcout << "PtgRef" <<std::endl;
// uint8_t ptg8 = 0, ptg = 0, PtgDataType = 0, null = 0;
//
// 2
Expand All @@ -1375,6 +1382,7 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgRef3d2:
case PtgRef3d3:
{
if (debug) Rcpp::Rcout << "PtgRef3d" <<std::endl;
// need_ptg_revextern = true;
RgbExtra typ = RevExtern;
ptgextra.push_back(typ);
Expand All @@ -1401,6 +1409,7 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgRefN2:
case PtgRefN3:
{
if (debug) Rcpp::Rcout << "PtgRefN" <<std::endl;

// A1 notation cell
fml_out += LocRel(sas, swapit, col, row);
Expand All @@ -1413,6 +1422,7 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgArea2:
case PtgArea3:
{
if (debug) Rcpp::Rcout << "PtgArea" <<std::endl;

// A1 notation cell
fml_out += Area(sas, swapit);
Expand All @@ -1425,6 +1435,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgArea3d2:
case PtgArea3d3:
{
if (debug) Rcpp::Rcout << "PtgArea3d" <<std::endl;

// need_ptg_revextern = true;
RgbExtra typ = RevExtern;
ptgextra.push_back(typ);
Expand All @@ -1451,6 +1463,7 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgAreaN2:
case PtgAreaN3:
{
if (debug) Rcpp::Rcout << "PtgAreaN" <<std::endl;

// A1 notation cell
fml_out += AreaRel(sas, swapit, col, row);
Expand All @@ -1463,6 +1476,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgMemArea2:
case PtgMemArea3:
{
if (debug) Rcpp::Rcout << "PtgMemArea" <<std::endl;

// need_ptg_extra_mem = true;
RgbExtra typ = PtgExtraMem;
ptgextra.push_back(typ);
Expand All @@ -1483,6 +1498,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgName2:
case PtgName3:
{
if (debug) Rcpp::Rcout << "PtgName" <<std::endl;

// need_ptg_revnametabid = true;
RgbExtra typ = RevNameTabid;
ptgextra.push_back(typ);
Expand All @@ -1499,6 +1516,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgNameX2:
case PtgNameX3:
{
if (debug) Rcpp::Rcout << "PtgNameX" <<std::endl;

// need_ptg_revname = true;
RgbExtra typ = RevName;
ptgextra.push_back(typ);
Expand All @@ -1519,6 +1538,7 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgRefErr2:
case PtgRefErr3:
{
if (debug) Rcpp::Rcout << "PtgRefErr" <<std::endl;

uint16_t unused2 = 0;
uint32_t unused1 = 0;
Expand All @@ -1536,6 +1556,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgRefErr3d2:
case PtgRefErr3d3:
{
if (debug) Rcpp::Rcout << "PtgRefErr3d" <<std::endl;

// need_ptg_revextern = true;
RgbExtra typ = RevExtern;
ptgextra.push_back(typ);
Expand Down Expand Up @@ -1563,6 +1585,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgAreaErr3d2:
case PtgAreaErr3d3:
{
if (debug) Rcpp::Rcout << "PtgAreaErr3d" <<std::endl;

// need_ptg_revextern = true;
RgbExtra typ = RevExtern;
ptgextra.push_back(typ);
Expand Down Expand Up @@ -1591,6 +1615,7 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgFunc2:
case PtgFunc3:
{
if (debug) Rcpp::Rcout << "PtgFunc" <<std::endl;

uint16_t iftab = 0;
iftab = readbin(iftab, sas, swapit);
Expand All @@ -1607,6 +1632,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgFuncVar2:
case PtgFuncVar3:
{
if (debug) Rcpp::Rcout << "PtgFuncVar" <<std::endl;

uint8_t cparams = 0, fCeFunc = 0; // number of parameters
cparams = readbin(cparams, sas, swapit);

Expand Down Expand Up @@ -1639,13 +1666,17 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co

case PtgErr:
{
if (debug) Rcpp::Rcout << "PtgErr" <<std::endl;

fml_out += BErr(sas, swapit);
fml_out += "\n";
break;
}

case PtgBool:
{
if (debug) Rcpp::Rcout << "PtgBool" <<std::endl;

int8_t boolean = 0;
boolean = readbin(boolean, sas, swapit);
fml_out += std::to_string(boolean);
Expand All @@ -1655,6 +1686,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co

case PtgExp:
{
if (debug) Rcpp::Rcout << "PtgExp" <<std::endl;

// this is a reference to the cell that contains the shared formula
uint32_t row = UncheckedRw(sas, swapit) + 1;
if (debug) Rcpp::Rcout << "PtgExp: " << row << std::endl;
Expand All @@ -1666,6 +1699,8 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
case PtgMemFunc2:
case PtgMemFunc3:
{
if (debug) Rcpp::Rcout << "PtgMemFunc" <<std::endl;

uint16_t cce = 0;
cce = readbin(cce, sas, swapit);
break;
Expand All @@ -1681,6 +1716,11 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co

}

if ((size_t)sas.tellg() != pos) {
// somethings not correct
sas.seekg(pos, sas.beg);
}

if (debug) Rcpp::Rcout << "--- formula ---\n" << fml_out << std::endl;

cb = readbin(cb, sas, swapit);
Expand All @@ -1700,11 +1740,14 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
while((size_t)sas.tellg() < pos) {

if (debug) Rcpp::Rcout << ".";
if (debug) Rprintf("Formula cb: %d\n", val1);
if (debug) {
Rprintf("Formula cb: %d\n", val1);
Rprintf("%d: %d\n", (size_t)sas.tellg(), pos);
}
// this is a little risky. maybe its some kind of vector indicating the
// order in which extra elements are going to be selected?

if (ptgextra.size() > 0 && ptgextra.size() >= cntr) {
if (ptgextra.size() > 0 && ptgextra.size() > cntr) {
if (ptgextra[cntr] == PtgExtraArray) {
if (debug) Rcpp::Rcout << "need PtgArray" << std::endl;
val1 = PtgArray;
Expand Down Expand Up @@ -1754,7 +1797,7 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
uint8_t reserved = 0;
reserved = readbin(reserved, sas, swapit);

std::string array;
std::string array = "";

if (debug) Rcpp::Rcout << (int32_t)reserved << std::endl;

Expand Down Expand Up @@ -1822,13 +1865,17 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
fml_out += "\n";
}

if (debug) Rcpp::Rcout << fml_out << std::endl;

break;
}

// do i need this?
case RevExtern:
{
sas.seekg(cb, sas.cur);
if (debug) Rcpp::Rcout << "RevExtern" << std::endl;

sas.seekg(pos, sas.beg);
break;

// I fear that I have to, but on another rainy day
Expand All @@ -1852,12 +1899,17 @@ std::string CellParsedFormula(std::istream& sas, bool swapit, bool debug, int co
{
// Rcpp::stop("Skip");
Rcpp::Rcout << "undefined cb: " << cb << std::endl;
sas.seekg(cb, sas.cur);
sas.seekg(pos, sas.beg);
break;
}
}
}

if ((size_t)sas.tellg() != pos) {
// somethings not correct
sas.seekg(pos, sas.beg);
}

if (debug) {
Rcpp::Rcout << "...fml..." << std::endl;
Rcpp::Rcout << fml_out << std::endl;
Expand Down

0 comments on commit 0471257

Please sign in to comment.