CodeSOD: Contains a Substring

One of the perks of open source software is that it means that large companies can and will patch it for their needs. Which means we can see what a particular large electronics vendor did with a video player application.

For example, they needed to see if the URL pointed to a stream protected by WideVine, Vudu, or Netflix. They can do this by checking if the filename contains a certain substring. Let's see how they accomplished this...

int get_special_protocol_type(char *filename, char *name){int type = 0;int fWidevine = 0;int j; char proto_str[2800] = {'\0', }; if (!strcmp("http", name)) {strcpy(proto_str, filename);for(j=0;proto_str[j] != '\0';j++){if(proto_str[j] == '='){j++;if(proto_str[j] == 'W'){j++;if(proto_str[j] == 'V'){type = Widevine_PROTOCOL;}}}}if (type == 0){for(j=0;proto_str[j] != '\0';j++){if(proto_str[j] == '='){j++;if(proto_str[j] == 'V'){j++;if(proto_str[j] == 'U'){j++;if(proto_str[j] == 'D'){j++;if(proto_str[j] == 'U'){type = VUDU_PROTOCOL;}}}}}}} if (type == 0) {for(j=0;proto_str[j] != '\0';j++){if(proto_str[j] == '='){j++;if(proto_str[j] == 'N'){j++;if(proto_str[j] == 'F'){j++;if(proto_str[j] == 'L'){j++;if(proto_str[j] == 'X'){type = Netflix_PROTOCOL;}}}}}}} }return type;}

For starters, there's been a lot of discussion around the importance of memory safe languages lately. I would argue that in C/C++ it's not actually hard to write memory safe code, it's just very easy not to. And this is an example- everything in here is a buffer overrun waiting to happen. The core problem is that we're passing pure pointers to char, and relying on the strings being properly null terminated. So we're using the old, unsafe string functions to never checking against the bounds of proto_str to make sure we haven't run off the edge. A malicious input could easily run off the end of the string.

But also, let's talk about that string comparison. They don't even just loop across a pair of strings character by character, they use this bizarre set of nested ifs with incrementing loop variables. Given that they use strcmp, I think we can safely assume the C standard library exists for their target, which means strnstr was right there.

It's also worth noting that, since this is a read-only operation, the strcpy is not necessary, though we're in a rough place since they're passing a pointer to char and not including the size, which gets us back to the whole "unsafe string operations" problem.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!