Cool VL Viewer forum

View unanswered posts | View active topics It is currently 2024-05-19 11:13:05



Reply to topic  [ 4 posts ] 
Theoretical out of bounds read in llavatarproperties.cpp 
Author Message

Joined: 2011-10-07 10:39:20
Posts: 185
Reply with quote
Another VC Analyzer warning:

If type > APT_NOTES (eg 6 or bigger), this logs an llerrs and then probably crashes on indexing
into the udp_methods array with index 4 or higher in the last line.

Code:
void LLAvatarProperties::sendGenericRequest(const LLUUID& avatar_id, S32 type)
{
   if (type != APT_AVATAR_INFO && (type < APT_GROUPS || type > APT_NOTES))
   {
      llerrs << "Invalid request type: " << type << llendl;
   }

   if (isPendingRequest(avatar_id, type))
   {
      LL_DEBUGS("AvatarProperties") << "Skipping duplicate request type "
                             << type << " for avatar " << avatar_id
                             << LL_ENDL;
      return;
   }

   if (type != APT_CLASSIFIEDS &&
      gSavedSettings.getBool("UseAgentProfileCap"))
   {
      std::string url = gAgent.getRegionCapability("AgentProfile");
      if (!url.empty())
      {
         LL_DEBUGS("AvatarProperties") << "Using AgentProfile capability to retrieve data for avatar: "
                                << avatar_id << LL_ENDL;
         addPendingRequest(avatar_id, APT_GROUPS);
         addPendingRequest(avatar_id, APT_PICKS);
         addPendingRequest(avatar_id, APT_NOTES);
         addPendingRequest(avatar_id, APT_AVATAR_INFO);
         url += "/" + avatar_id.asString();
         gCoros.launch("requestAgentUserInfoCoro",
                    boost::bind(requestAvatarPropertiesCoro, avatar_id,
                             url));
         // Also request an agent groups list refresh for LLAgent. HB
         if (avatar_id == gAgentID)
         {
            gAgent.sendAgentDataUpdateRequest();
         }
         return;
      }
   }

   if (type == APT_AVATAR_INFO)
   {
      sendAvatarPropertiesRequest(avatar_id);
      return;
   }

   addPendingRequest(avatar_id, type);

   // Must match the order defined in EAvatarPropertiesUpdateType
   static const char* udp_methods[] =
   {
      "avatargroupsrequest",      // APT_GROUPS
      "avatarpicksrequest",      // APT_PICKS
      "avatarclassifiedsrequest",   // APT_CLASSIFIEDS
      "avatarnotesrequest",      // APT_NOTES
   };
   const char* method = udp_methods[type - APT_GROUPS];


2024-05-06 14:45:12
Profile

Joined: 2009-03-17 18:42:51
Posts: 5587
Reply with quote
No bug here: the analyzer is simply unaware that llerrs stops the viewer execution altogether by voluntarily crashing it... :lol:

This clause should actually never be executed, unless the programmer (me) invokes the sendGenericRequest() method with a bad request type in their code (and this would get caught thanks to llerrs during the testing after such a bogus code change).

Of course, I could add a totally pointless "return;" after llerrs... But what for ?...


2024-05-06 14:48:26
Profile WWW

Joined: 2011-10-07 10:39:20
Posts: 185
Reply with quote
Ok, overlooked the llerrs abort as well.
And yes, sometimes the analyzer is too stupid, had another 2-3 false positives already thrown out.

Maybe llerrs would be a candidate for [[noreturn]] attributes ?
https://en.cppreference.com/w/cpp/langu ... s/noreturn


2024-05-06 14:54:29
Profile

Joined: 2009-03-17 18:42:51
Posts: 5587
Reply with quote
kathrine wrote:
Maybe llerrs would be a candidate for [[noreturn]] attributes ?
This won't work the way llerrs (a preprocessor macro) is declared and expands in an if() test and a std::ostringstream expression (i.e. this is not a function and [[noreturn]] cannot apply here), the stream and the if() test block being closed in the llendl macro.

I would instead need to add a special llerrs_endl macro to replace llendl when used together with llerrs, adding an explicit return in that macro, but it is just too complicated for the sole purpose of avoiding a stupid code analyzer warning... :P


2024-05-06 15:03:58
Profile WWW
Display posts from previous:  Sort by  
Reply to topic   [ 4 posts ] 

Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
cron
Powered by phpBB® Forum Software © phpBB Group
Designed by ST Software.