[tor-bugs] #17868 [Core Tor/Tor]: base64_decode_nopad() destination buffer length problem

[Tor Bug Tracker & Wiki]

#17868: base64_decode_nopad() destination buffer length problem
--------------------------+------------------------------------
 Reporter:  dgoulet       |          Owner:  nikkolasg
     Type:  defect        |         Status:  needs_revision
 Priority:  Medium        |      Milestone:  Tor: 0.2.9.x-final
Component:  Core Tor/Tor  |        Version:
 Severity:  Normal        |     Resolution:
 Keywords:                |  Actual Points:
Parent ID:                |         Points:  0.1
 Reviewer:  dgoulet       |        Sponsor:  SponsorR-can
--------------------------+------------------------------------
Changes (by andrea):

 * status:  needs_review => needs_revision


Comment:

 Review of
 https://trac.torproject.org/projects/tor/attachment/ticket/17868/base64_decode_nopad_reviewed1.patch
 for #17868:

 (patch is in my ticket17868-nikkolasg-patch-v3 branch for reference/future
 work)

 Substantive points:

  - I believe the copy/add padding on the source dance in
 base64_decode_nopad()
    is unnecessary, since base64_decode_internal() just falls out of the
 loop
    when it sees a padding character and will behave identically if called
    with n % 4 != 0 non-padding inputs in src.

  - if base64_decode_internal() is not supposed to be called from outside
    util_format.c, it should be static and declared in util_format.c rather
    than util_format.h, or perhaps STATIC and in an #ifdef-ed off private
    section of util_format.h if the test suite needs to see it.

  - It might be simpler, especially given the possibility of space
 characters
    in the middle of the input, to just check dest against dest_len at each
    write in base64_decode_internal() and error-exit if we run out of
 buffer
    rather than trying to precalculate output sizes.

 Style/spelling/etc.:

  - s/responsability/responsibility/ in comment for
 base64_decode_internal()

  - please fix the alignment of args to base64_decode_internal()

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/17868#comment:15>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online