[SERVER-68235] Coverity analysis defect 123308: Untrusted loop bound Created: 22/Jul/22  Updated: 05/Dec/22  Resolved: 01/Aug/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Coverity Collector User Assignee: Backlog - Service Architecture
Resolution: Won't Fix Votes: 0
Labels: coverity
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File image-2022-07-25-15-43-12-745.png    
Assigned Teams:
Service Arch
Operating System: ALL
Participants:

 Description   

Untrusted loop bound

An attacker could control the number of times the loop iterates. An unscrutinized value from an untrusted source used as a loop bound
/src/mongo/transport/transport_layer_asio.cpp:990: TAINTED_SCALAR 123308 Calling function "operator >>" taints argument "val". [Note: The source code implementation of the function has been overridden by a builtin model.]
/src/mongo/transport/transport_layer_asio.cpp:997: TAINTED_SCALAR 123308 Assigning: "wantval" = "val". Both are now tainted.
/src/mongo/transport/transport_layer_asio.cpp:1006: TAINTED_SCALAR 123308 Checking lower bounds of signed scalar "wantval" by taking the true branch of "wantval > 9L".



 Comments   
Comment by Jason Chan [ 01/Aug/22 ]

Set to ignore in coverity

Comment by Eric Milkie [ 25/Jul/22 ]

Sadly, "Service Unavailable" is exactly the message that is displayed if you fail to authenticate to the Okta LDAP plugin before it times out..

Comment by Billy Donahue [ 25/Jul/22 ]

I'm not failing to authenticate. When I do authenticate, the coverity server displays a page indicating that it is unavailable.

"Service Unavailable
The service is temporarily unavailable. Please try again later."

Anyway this ticket is not a safety concern and we should close it.

Comment by Eric Milkie [ 25/Jul/22 ]

You have to have an Okta 2FA push method configured to log into Coverity (like the Okta Verify app).   (or else put your Okta 2FA six digit code after your password, separated by a comma)

I'm not sure how to untaint something like this and I can't find any help on Google.  I guess we should just ignore this defect and close the ticket?

Comment by Billy Donahue [ 25/Jul/22 ]

I'll take your word for it since I can't log in to Coverity without throwing an internal server error.

So yeah it's the tainted integer controlling how many digits its string representation has.
If we want to say that ALL values of `val` are permissible, how do we do that?

Comment by Eric Milkie [ 25/Jul/22 ]

  // Write an unsigned integer value to the range [first,first+len).
  // The caller is required to provide a buffer of exactly the right size
  // (which can be determined by the __to_chars_len function).
  template<typename _Tp>
    void
    __to_chars_10_impl(char* __first, unsigned __len, _Tp __val) noexcept
    {
      static_assert(is_integral<_Tp>::value, "implementation bug");
      static_assert(is_unsigned<_Tp>::value, "implementation bug");      static constexpr char __digits[201] =
        "0001020304050607080910111213141516171819"
        "2021222324252627282930313233343536373839"
        "4041424344454647484950515253545556575859"
        "6061626364656667686970717273747576777879"
        "8081828384858687888990919293949596979899";
      unsigned __pos = __len - 1;
      while (__val >= 100)
        {
          auto const __num = (__val % 100) * 2;
          __val /= 100;
          __first[__pos] = __digits[__num + 1];
          __first[__pos - 1] = __digits[__num];
          __pos -= 2;
        }
      if (__val >= 10)
        {
          auto const __num = __val * 2;
          __first[1] = __digits[__num + 1];
          __first[0] = __digits[__num];
        }
      else
        __first[0] = '0' + __val;
    }
 

Comment by Eric Milkie [ 25/Jul/22 ]

If you click into the defect and click show-details, it will show you the loop:

Comment by Billy Donahue [ 25/Jul/22 ]

So the way to remove the taint is to add an assertion on the value, so it goes from being an unscrutinized value to a scrutinized value.

I don't see a loop controlled by the tainted integer though?
Do they mean a loop inside of std::to_string(wantval) ?
Like, the number of digits?

Generated at Thu Feb 08 06:10:16 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.